| Message ID | 1573578324-8389-12-git-send-email-kwankhede@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add migration support for VFIO devices | expand |
On Tue, 12 Nov 2019 22:35:20 +0530 Kirti Wankhede <kwankhede@nvidia.com> wrote: > Added .save_live_pending, .save_live_iterate and .save_live_complete_precopy > functions. These functions handles pre-copy and stop-and-copy phase. > > In _SAVING|_RUNNING device state or pre-copy phase: > - read pending_bytes. If pending_bytes > 0, go through below steps. > - read data_offset - indicates kernel driver to write data to staging > buffer. > - read data_size - amount of data in bytes written by vendor driver in > migration region. > - read data_size bytes of data from data_offset in the migration region. > - Write data packet to file stream as below: > {VFIO_MIG_FLAG_DEV_DATA_STATE, data_size, actual data, > VFIO_MIG_FLAG_END_OF_STATE } > > In _SAVING device state or stop-and-copy phase > a. read config space of device and save to migration file stream. This > doesn't need to be from vendor driver. Any other special config state > from driver can be saved as data in following iteration. > b. read pending_bytes. If pending_bytes > 0, go through below steps. > c. read data_offset - indicates kernel driver to write data to staging > buffer. > d. read data_size - amount of data in bytes written by vendor driver in > migration region. > e. read data_size bytes of data from data_offset in the migration region. > f. Write data packet as below: > {VFIO_MIG_FLAG_DEV_DATA_STATE, data_size, actual data} > g. iterate through steps b to f while (pending_bytes > 0) > h. Write {VFIO_MIG_FLAG_END_OF_STATE} > > When data region is mapped, its user's responsibility to read data from s/mapped/made available/ "mapped" is confusing given the mmap'd features. > data_offset of data_size before moving to next steps. > > Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com> > Reviewed-by: Neo Jia <cjia@nvidia.com> > --- > hw/vfio/migration.c | 245 ++++++++++++++++++++++++++++++++++++++++++++++++++- > hw/vfio/trace-events | 6 ++ > 2 files changed, 250 insertions(+), 1 deletion(-) > > diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c > index 48aac6d29876..f890e864e174 100644 > --- a/hw/vfio/migration.c > +++ b/hw/vfio/migration.c > @@ -120,6 +120,137 @@ static int vfio_migration_set_state(VFIODevice *vbasedev, uint32_t set_flags, > return 0; > } > > +static void *find_data_region(VFIORegion *region, > + uint64_t data_offset, > + uint64_t data_size) > +{ > + void *ptr = NULL; > + int i; > + > + for (i = 0; i < region->nr_mmaps; i++) { > + if ((data_offset >= region->mmaps[i].offset) && > + (data_offset < region->mmaps[i].offset + region->mmaps[i].size) && > + (data_size <= region->mmaps[i].size)) { data_offset is determined to live somewhere within the mmap and data_size is independently determined to be smaller than the entire mmaps size. This is broken. > + ptr = region->mmaps[i].mmap + (data_offset - > + region->mmaps[i].offset); If the data offset is mmap'd, this gives us a pointer to the start, but we have no idea if the entire range is accessible via this pointer, nor does the API require it to be. > + break; > + } > + } > + return ptr; > +} > + > +static int vfio_save_buffer(QEMUFile *f, VFIODevice *vbasedev) > +{ > + VFIOMigration *migration = vbasedev->migration; > + VFIORegion *region = &migration->region; > + uint64_t data_offset = 0, data_size = 0; > + int ret; > + > + ret = pread(vbasedev->fd, &data_offset, sizeof(data_offset), > + region->fd_offset + offsetof(struct vfio_device_migration_info, > + data_offset)); > + if (ret != sizeof(data_offset)) { > + error_report("%s: Failed to get migration buffer data offset %d", > + vbasedev->name, ret); > + return -EINVAL; > + } > + > + ret = pread(vbasedev->fd, &data_size, sizeof(data_size), > + region->fd_offset + offsetof(struct vfio_device_migration_info, > + data_size)); > + if (ret != sizeof(data_size)) { > + error_report("%s: Failed to get migration buffer data size %d", > + vbasedev->name, ret); > + return -EINVAL; > + } > + > + if (data_size > 0) { > + void *buf = NULL; > + bool buffer_mmaped; > + > + if (region->mmaps) { > + buf = find_data_region(region, data_offset, data_size); > + } > + > + buffer_mmaped = (buf != NULL) ? true : false; > + > + if (!buffer_mmaped) { > + buf = g_try_malloc0(data_size); > + if (!buf) { > + error_report("%s: Error allocating buffer ", __func__); > + return -ENOMEM; > + } > + > + ret = pread(vbasedev->fd, buf, data_size, > + region->fd_offset + data_offset); > + if (ret != data_size) { > + error_report("%s: Failed to get migration data %d", > + vbasedev->name, ret); > + g_free(buf); > + return -EINVAL; > + } Seems like this needs to be bound and chunked to some "reasonable" size, the vendor driver could be exposing gigabytes of data at a time. Do we have a quota for this phase? > + } > + > + qemu_put_be64(f, data_size); > + qemu_put_buffer(f, buf, data_size); > + > + if (!buffer_mmaped) { > + g_free(buf); > + } > + } else { > + qemu_put_be64(f, data_size); Why would we stuff zero or <0 into the data stream here? > + } There's clearly an assumption here that a chunk of data from the device is entirely mmap'd or not. That's not a requirement of the API and I believe we've discussed cases where a vendor driver could have partially mapped and partially emulated data chunks. > + > + trace_vfio_save_buffer(vbasedev->name, data_offset, data_size, > + migration->pending_bytes); > + > + ret = qemu_file_get_error(f); > + if (ret) { > + return ret; > + } > + > + return data_size; AIUI, it's required to read pending_bytes to indicate the data has been consumed, I don't see that done here. > +} > + > +static int vfio_update_pending(VFIODevice *vbasedev) > +{ > + VFIOMigration *migration = vbasedev->migration; > + VFIORegion *region = &migration->region; > + uint64_t pending_bytes = 0; > + int ret; > + > + ret = pread(vbasedev->fd, &pending_bytes, sizeof(pending_bytes), > + region->fd_offset + offsetof(struct vfio_device_migration_info, > + pending_bytes)); > + if ((ret < 0) || (ret != sizeof(pending_bytes))) { Two tests for the same thing. > + error_report("%s: Failed to get pending bytes %d", > + vbasedev->name, ret); > + migration->pending_bytes = 0; > + return (ret < 0) ? ret : -EINVAL; pread returns -1 on error and sets errno, the ret value isn't very interesting. > + } > + > + migration->pending_bytes = pending_bytes; > + trace_vfio_update_pending(vbasedev->name, pending_bytes); > + return 0; > +} > + > +static int vfio_save_device_config_state(QEMUFile *f, void *opaque) > +{ > + VFIODevice *vbasedev = opaque; > + > + qemu_put_be64(f, VFIO_MIG_FLAG_DEV_CONFIG_STATE); > + > + if (vbasedev->ops && vbasedev->ops->vfio_save_config) { A vbasedev w/o ops doesn't seem possible. > + vbasedev->ops->vfio_save_config(vbasedev, f); > + } > + > + qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE); Is it worthwhile to include these markers w/o vfio_save_config? Couldn't we simply move them into the conditional section? > + > + trace_vfio_save_device_config_state(vbasedev->name); > + > + return qemu_file_get_error(f); > +} > + > /* ---------------------------------------------------------------------- */ > > static int vfio_save_setup(QEMUFile *f, void *opaque) > @@ -136,7 +267,7 @@ static int vfio_save_setup(QEMUFile *f, void *opaque) > qemu_mutex_unlock_iothread(); > if (ret) { > error_report("%s: Failed to mmap VFIO migration region %d: %s", > - vbasedev->name, migration->region.index, > + vbasedev->name, migration->region.nr, Fix this in patch that introduced it, looks like this wouldn't even compile previously. > strerror(-ret)); > return ret; > } > @@ -170,9 +301,121 @@ static void vfio_save_cleanup(void *opaque) > trace_vfio_save_cleanup(vbasedev->name); > } > > +static void vfio_save_pending(QEMUFile *f, void *opaque, > + uint64_t threshold_size, > + uint64_t *res_precopy_only, > + uint64_t *res_compatible, > + uint64_t *res_postcopy_only) > +{ > + VFIODevice *vbasedev = opaque; > + VFIOMigration *migration = vbasedev->migration; > + int ret; > + > + ret = vfio_update_pending(vbasedev); > + if (ret) { > + return; > + } > + > + *res_precopy_only += migration->pending_bytes; > + > + trace_vfio_save_pending(vbasedev->name, *res_precopy_only, > + *res_postcopy_only, *res_compatible); > +} > + > +static int vfio_save_iterate(QEMUFile *f, void *opaque) > +{ > + VFIODevice *vbasedev = opaque; > + int ret, data_size; > + > + qemu_put_be64(f, VFIO_MIG_FLAG_DEV_DATA_STATE); > + > + data_size = vfio_save_buffer(f, vbasedev); > + > + if (data_size < 0) { > + error_report("%s: vfio_save_buffer failed %s", vbasedev->name, > + strerror(errno)); > + return data_size; > + } > + > + qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE); > + > + ret = qemu_file_get_error(f); > + if (ret) { > + return ret; > + } > + > + trace_vfio_save_iterate(vbasedev->name, data_size); > + if (data_size == 0) { > + /* indicates data finished, goto complete phase */ > + return 1; > + } I don't think this matches our spec, pending_bytes seems to indicate we've finished, data_size of zero does not have a defined meaning. > + > + return 0; > +} > + > +static int vfio_save_complete_precopy(QEMUFile *f, void *opaque) > +{ > + VFIODevice *vbasedev = opaque; > + VFIOMigration *migration = vbasedev->migration; > + int ret; > + > + ret = vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_SAVING, > + VFIO_DEVICE_STATE_RUNNING); > + if (ret) { > + error_report("%s: Failed to set state STOP and SAVING", > + vbasedev->name); > + return ret; > + } > + > + ret = vfio_save_device_config_state(f, opaque); > + if (ret) { > + return ret; > + } > + > + ret = vfio_update_pending(vbasedev); > + if (ret) { > + return ret; > + } > + > + while (migration->pending_bytes > 0) { > + qemu_put_be64(f, VFIO_MIG_FLAG_DEV_DATA_STATE); > + ret = vfio_save_buffer(f, vbasedev); > + if (ret < 0) { > + error_report("%s: Failed to save buffer", vbasedev->name); > + return ret; > + } else if (ret == 0) { > + break; > + } > + > + ret = vfio_update_pending(vbasedev); > + if (ret) { > + return ret; > + } > + } Here we use pending_bytes for the stop condition and update pending_bytes per loop, which is different than the above save iterate function. > + > + qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE); > + > + ret = qemu_file_get_error(f); > + if (ret) { > + return ret; > + } > + > + ret = vfio_migration_set_state(vbasedev, 0, VFIO_DEVICE_STATE_SAVING); > + if (ret) { > + error_report("%s: Failed to set state STOPPED", vbasedev->name); > + return ret; > + } > + > + trace_vfio_save_complete_precopy(vbasedev->name); > + return ret; > +} > + > static SaveVMHandlers savevm_vfio_handlers = { > .save_setup = vfio_save_setup, > .save_cleanup = vfio_save_cleanup, > + .save_live_pending = vfio_save_pending, > + .save_live_iterate = vfio_save_iterate, > + .save_live_complete_precopy = vfio_save_complete_precopy, > }; > > /* ---------------------------------------------------------------------- */ > diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events > index 4bb43f18f315..bdf40ba368c7 100644 > --- a/hw/vfio/trace-events > +++ b/hw/vfio/trace-events > @@ -151,3 +151,9 @@ vfio_vmstate_change(char *name, int running, const char *reason, uint32_t dev_st > vfio_migration_state_notifier(char *name, int state) " (%s) state %d" > vfio_save_setup(char *name) " (%s)" > vfio_save_cleanup(char *name) " (%s)" > +vfio_save_buffer(char *name, uint64_t data_offset, uint64_t data_size, uint64_t pending) " (%s) Offset 0x%"PRIx64" size 0x%"PRIx64" pending 0x%"PRIx64 > +vfio_update_pending(char *name, uint64_t pending) " (%s) pending 0x%"PRIx64 > +vfio_save_device_config_state(char *name) " (%s)" > +vfio_save_pending(char *name, uint64_t precopy, uint64_t postcopy, uint64_t compatible) " (%s) precopy 0x%"PRIx64" postcopy 0x%"PRIx64" compatible 0x%"PRIx64 > +vfio_save_iterate(char *name, int data_size) " (%s) data_size %d" > +vfio_save_complete_precopy(char *name) " (%s)"
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c index 48aac6d29876..f890e864e174 100644 --- a/hw/vfio/migration.c +++ b/hw/vfio/migration.c @@ -120,6 +120,137 @@ static int vfio_migration_set_state(VFIODevice *vbasedev, uint32_t set_flags, return 0; } +static void *find_data_region(VFIORegion *region, + uint64_t data_offset, + uint64_t data_size) +{ + void *ptr = NULL; + int i; + + for (i = 0; i < region->nr_mmaps; i++) { + if ((data_offset >= region->mmaps[i].offset) && + (data_offset < region->mmaps[i].offset + region->mmaps[i].size) && + (data_size <= region->mmaps[i].size)) { + ptr = region->mmaps[i].mmap + (data_offset - + region->mmaps[i].offset); + break; + } + } + return ptr; +} + +static int vfio_save_buffer(QEMUFile *f, VFIODevice *vbasedev) +{ + VFIOMigration *migration = vbasedev->migration; + VFIORegion *region = &migration->region; + uint64_t data_offset = 0, data_size = 0; + int ret; + + ret = pread(vbasedev->fd, &data_offset, sizeof(data_offset), + region->fd_offset + offsetof(struct vfio_device_migration_info, + data_offset)); + if (ret != sizeof(data_offset)) { + error_report("%s: Failed to get migration buffer data offset %d", + vbasedev->name, ret); + return -EINVAL; + } + + ret = pread(vbasedev->fd, &data_size, sizeof(data_size), + region->fd_offset + offsetof(struct vfio_device_migration_info, + data_size)); + if (ret != sizeof(data_size)) { + error_report("%s: Failed to get migration buffer data size %d", + vbasedev->name, ret); + return -EINVAL; + } + + if (data_size > 0) { + void *buf = NULL; + bool buffer_mmaped; + + if (region->mmaps) { + buf = find_data_region(region, data_offset, data_size); + } + + buffer_mmaped = (buf != NULL) ? true : false; + + if (!buffer_mmaped) { + buf = g_try_malloc0(data_size); + if (!buf) { + error_report("%s: Error allocating buffer ", __func__); + return -ENOMEM; + } + + ret = pread(vbasedev->fd, buf, data_size, + region->fd_offset + data_offset); + if (ret != data_size) { + error_report("%s: Failed to get migration data %d", + vbasedev->name, ret); + g_free(buf); + return -EINVAL; + } + } + + qemu_put_be64(f, data_size); + qemu_put_buffer(f, buf, data_size); + + if (!buffer_mmaped) { + g_free(buf); + } + } else { + qemu_put_be64(f, data_size); + } + + trace_vfio_save_buffer(vbasedev->name, data_offset, data_size, + migration->pending_bytes); + + ret = qemu_file_get_error(f); + if (ret) { + return ret; + } + + return data_size; +} + +static int vfio_update_pending(VFIODevice *vbasedev) +{ + VFIOMigration *migration = vbasedev->migration; + VFIORegion *region = &migration->region; + uint64_t pending_bytes = 0; + int ret; + + ret = pread(vbasedev->fd, &pending_bytes, sizeof(pending_bytes), + region->fd_offset + offsetof(struct vfio_device_migration_info, + pending_bytes)); + if ((ret < 0) || (ret != sizeof(pending_bytes))) { + error_report("%s: Failed to get pending bytes %d", + vbasedev->name, ret); + migration->pending_bytes = 0; + return (ret < 0) ? ret : -EINVAL; + } + + migration->pending_bytes = pending_bytes; + trace_vfio_update_pending(vbasedev->name, pending_bytes); + return 0; +} + +static int vfio_save_device_config_state(QEMUFile *f, void *opaque) +{ + VFIODevice *vbasedev = opaque; + + qemu_put_be64(f, VFIO_MIG_FLAG_DEV_CONFIG_STATE); + + if (vbasedev->ops && vbasedev->ops->vfio_save_config) { + vbasedev->ops->vfio_save_config(vbasedev, f); + } + + qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE); + + trace_vfio_save_device_config_state(vbasedev->name); + + return qemu_file_get_error(f); +} + /* ---------------------------------------------------------------------- */ static int vfio_save_setup(QEMUFile *f, void *opaque) @@ -136,7 +267,7 @@ static int vfio_save_setup(QEMUFile *f, void *opaque) qemu_mutex_unlock_iothread(); if (ret) { error_report("%s: Failed to mmap VFIO migration region %d: %s", - vbasedev->name, migration->region.index, + vbasedev->name, migration->region.nr, strerror(-ret)); return ret; } @@ -170,9 +301,121 @@ static void vfio_save_cleanup(void *opaque) trace_vfio_save_cleanup(vbasedev->name); } +static void vfio_save_pending(QEMUFile *f, void *opaque, + uint64_t threshold_size, + uint64_t *res_precopy_only, + uint64_t *res_compatible, + uint64_t *res_postcopy_only) +{ + VFIODevice *vbasedev = opaque; + VFIOMigration *migration = vbasedev->migration; + int ret; + + ret = vfio_update_pending(vbasedev); + if (ret) { + return; + } + + *res_precopy_only += migration->pending_bytes; + + trace_vfio_save_pending(vbasedev->name, *res_precopy_only, + *res_postcopy_only, *res_compatible); +} + +static int vfio_save_iterate(QEMUFile *f, void *opaque) +{ + VFIODevice *vbasedev = opaque; + int ret, data_size; + + qemu_put_be64(f, VFIO_MIG_FLAG_DEV_DATA_STATE); + + data_size = vfio_save_buffer(f, vbasedev); + + if (data_size < 0) { + error_report("%s: vfio_save_buffer failed %s", vbasedev->name, + strerror(errno)); + return data_size; + } + + qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE); + + ret = qemu_file_get_error(f); + if (ret) { + return ret; + } + + trace_vfio_save_iterate(vbasedev->name, data_size); + if (data_size == 0) { + /* indicates data finished, goto complete phase */ + return 1; + } + + return 0; +} + +static int vfio_save_complete_precopy(QEMUFile *f, void *opaque) +{ + VFIODevice *vbasedev = opaque; + VFIOMigration *migration = vbasedev->migration; + int ret; + + ret = vfio_migration_set_state(vbasedev, VFIO_DEVICE_STATE_SAVING, + VFIO_DEVICE_STATE_RUNNING); + if (ret) { + error_report("%s: Failed to set state STOP and SAVING", + vbasedev->name); + return ret; + } + + ret = vfio_save_device_config_state(f, opaque); + if (ret) { + return ret; + } + + ret = vfio_update_pending(vbasedev); + if (ret) { + return ret; + } + + while (migration->pending_bytes > 0) { + qemu_put_be64(f, VFIO_MIG_FLAG_DEV_DATA_STATE); + ret = vfio_save_buffer(f, vbasedev); + if (ret < 0) { + error_report("%s: Failed to save buffer", vbasedev->name); + return ret; + } else if (ret == 0) { + break; + } + + ret = vfio_update_pending(vbasedev); + if (ret) { + return ret; + } + } + + qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE); + + ret = qemu_file_get_error(f); + if (ret) { + return ret; + } + + ret = vfio_migration_set_state(vbasedev, 0, VFIO_DEVICE_STATE_SAVING); + if (ret) { + error_report("%s: Failed to set state STOPPED", vbasedev->name); + return ret; + } + + trace_vfio_save_complete_precopy(vbasedev->name); + return ret; +} + static SaveVMHandlers savevm_vfio_handlers = { .save_setup = vfio_save_setup, .save_cleanup = vfio_save_cleanup, + .save_live_pending = vfio_save_pending, + .save_live_iterate = vfio_save_iterate, + .save_live_complete_precopy = vfio_save_complete_precopy, }; /* ---------------------------------------------------------------------- */ diff --git a/hw/vfio/trace-events b/hw/vfio/trace-events index 4bb43f18f315..bdf40ba368c7 100644 --- a/hw/vfio/trace-events +++ b/hw/vfio/trace-events @@ -151,3 +151,9 @@ vfio_vmstate_change(char *name, int running, const char *reason, uint32_t dev_st vfio_migration_state_notifier(char *name, int state) " (%s) state %d" vfio_save_setup(char *name) " (%s)" vfio_save_cleanup(char *name) " (%s)" +vfio_save_buffer(char *name, uint64_t data_offset, uint64_t data_size, uint64_t pending) " (%s) Offset 0x%"PRIx64" size 0x%"PRIx64" pending 0x%"PRIx64 +vfio_update_pending(char *name, uint64_t pending) " (%s) pending 0x%"PRIx64 +vfio_save_device_config_state(char *name) " (%s)" +vfio_save_pending(char *name, uint64_t precopy, uint64_t postcopy, uint64_t compatible) " (%s) precopy 0x%"PRIx64" postcopy 0x%"PRIx64" compatible 0x%"PRIx64 +vfio_save_iterate(char *name, int data_size) " (%s) data_size %d" +vfio_save_complete_precopy(char *name) " (%s)"
Added .save_live_pending, .save_live_iterate and .save_live_complete_precopy functions. These functions handles pre-copy and stop-and-copy phase. In _SAVING|_RUNNING device state or pre-copy phase: - read pending_bytes. If pending_bytes > 0, go through below steps. - read data_offset - indicates kernel driver to write data to staging buffer. - read data_size - amount of data in bytes written by vendor driver in migration region. - read data_size bytes of data from data_offset in the migration region. - Write data packet to file stream as below: {VFIO_MIG_FLAG_DEV_DATA_STATE, data_size, actual data, VFIO_MIG_FLAG_END_OF_STATE } In _SAVING device state or stop-and-copy phase a. read config space of device and save to migration file stream. This doesn't need to be from vendor driver. Any other special config state from driver can be saved as data in following iteration. b. read pending_bytes. If pending_bytes > 0, go through below steps. c. read data_offset - indicates kernel driver to write data to staging buffer. d. read data_size - amount of data in bytes written by vendor driver in migration region. e. read data_size bytes of data from data_offset in the migration region. f. Write data packet as below: {VFIO_MIG_FLAG_DEV_DATA_STATE, data_size, actual data} g. iterate through steps b to f while (pending_bytes > 0) h. Write {VFIO_MIG_FLAG_END_OF_STATE} When data region is mapped, its user's responsibility to read data from data_offset of data_size before moving to next steps. Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com> Reviewed-by: Neo Jia <cjia@nvidia.com> --- hw/vfio/migration.c | 245 ++++++++++++++++++++++++++++++++++++++++++++++++++- hw/vfio/trace-events | 6 ++ 2 files changed, 250 insertions(+), 1 deletion(-)