[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] vfio/migrate: Move switch of dirty tracking into vfio_memory
|
From: |
Alex Williamson |
|
Subject: |
Re: [PATCH] vfio/migrate: Move switch of dirty tracking into vfio_memory_listener |
|
Date: |
Tue, 26 Jan 2021 15:29:39 -0700 |
Kirti? Migration experts? Thanks,
Alex
On Mon, 11 Jan 2021 15:34:39 +0800
Keqian Zhu <zhukeqian1@huawei.com> wrote:
> For now the switch of vfio dirty page tracking is integrated into
> the vfio_save_handler, it causes some problems [1].
>
> The object of dirty tracking is guest memory, but the object of
> the vfio_save_handler is device state. This mixed logic produces
> unnecessary coupling and conflicts:
>
> 1. Coupling: Their saving granule is different (perVM vs perDevice).
> vfio will enable dirty_page_tracking for each devices, actually
> once is enough.
> 2. Conflicts: The ram_save_setup() traverses all memory_listeners
> to execute their log_start() and log_sync() hooks to get the
> first round dirty bitmap, which is used by the bulk stage of
> ram saving. However, it can't get dirty bitmap from vfio, as
> @savevm_ram_handlers is registered before @vfio_save_handler.
>
> Move the switch of vfio dirty_page_tracking into vfio_memory_listener
> can solve above problems. Besides, Do not require devices in SAVING
> state for vfio_sync_dirty_bitmap().
>
> [1] https://www.spinics.net/lists/kvm/msg229967.html
>
> Reported-by: Zenghui Yu <yuzenghui@huawei.com>
> Signed-off-by: Keqian Zhu <zhukeqian1@huawei.com>
> ---
> hw/vfio/common.c | 53 +++++++++++++++++++++++++++++++++++++--------
> hw/vfio/migration.c | 35 ------------------------------
> 2 files changed, 44 insertions(+), 44 deletions(-)
>
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 6ff1daa763..9128cd7ee1 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -311,7 +311,7 @@ bool vfio_mig_active(void)
> return true;
> }
>
> -static bool vfio_devices_all_saving(VFIOContainer *container)
> +static bool vfio_devices_all_dirty_tracking(VFIOContainer *container)
> {
> VFIOGroup *group;
> VFIODevice *vbasedev;
> @@ -329,13 +329,8 @@ static bool vfio_devices_all_saving(VFIOContainer
> *container)
> return false;
> }
>
> - if (migration->device_state & VFIO_DEVICE_STATE_SAVING) {
> - if ((vbasedev->pre_copy_dirty_page_tracking ==
> ON_OFF_AUTO_OFF)
> - && (migration->device_state &
> VFIO_DEVICE_STATE_RUNNING)) {
> - return false;
> - }
> - continue;
> - } else {
> + if ((vbasedev->pre_copy_dirty_page_tracking == ON_OFF_AUTO_OFF)
> + && (migration->device_state & VFIO_DEVICE_STATE_RUNNING)) {
> return false;
> }
> }
> @@ -987,6 +982,44 @@ static void vfio_listener_region_del(MemoryListener
> *listener,
> }
> }
>
> +static void vfio_set_dirty_page_tracking(VFIOContainer *container, bool
> start)
> +{
> + int ret;
> + struct vfio_iommu_type1_dirty_bitmap dirty = {
> + .argsz = sizeof(dirty),
> + };
> +
> + if (start) {
> + dirty.flags = VFIO_IOMMU_DIRTY_PAGES_FLAG_START;
> + } else {
> + dirty.flags = VFIO_IOMMU_DIRTY_PAGES_FLAG_STOP;
> + }
> +
> + ret = ioctl(container->fd, VFIO_IOMMU_DIRTY_PAGES, &dirty);
> + if (ret) {
> + error_report("Failed to set dirty tracking flag 0x%x errno: %d",
> + dirty.flags, errno);
> + }
> +}
> +
> +static void vfio_listener_log_start(MemoryListener *listener,
> + MemoryRegionSection *section,
> + int old, int new)
> +{
> + VFIOContainer *container = container_of(listener, VFIOContainer,
> listener);
> +
> + vfio_set_dirty_page_tracking(container, true);
> +}
> +
> +static void vfio_listener_log_stop(MemoryListener *listener,
> + MemoryRegionSection *section,
> + int old, int new)
> +{
> + VFIOContainer *container = container_of(listener, VFIOContainer,
> listener);
> +
> + vfio_set_dirty_page_tracking(container, false);
> +}
> +
> static int vfio_get_dirty_bitmap(VFIOContainer *container, uint64_t iova,
> uint64_t size, ram_addr_t ram_addr)
> {
> @@ -1128,7 +1161,7 @@ static void vfio_listerner_log_sync(MemoryListener
> *listener,
> return;
> }
>
> - if (vfio_devices_all_saving(container)) {
> + if (vfio_devices_all_dirty_tracking(container)) {
> vfio_sync_dirty_bitmap(container, section);
> }
> }
> @@ -1136,6 +1169,8 @@ static void vfio_listerner_log_sync(MemoryListener
> *listener,
> static const MemoryListener vfio_memory_listener = {
> .region_add = vfio_listener_region_add,
> .region_del = vfio_listener_region_del,
> + .log_start = vfio_listener_log_start,
> + .log_stop = vfio_listener_log_stop,
> .log_sync = vfio_listerner_log_sync,
> };
>
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> index 00daa50ed8..c0f646823a 100644
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
> @@ -395,40 +395,10 @@ static int vfio_load_device_config_state(QEMUFile *f,
> void *opaque)
> return qemu_file_get_error(f);
> }
>
> -static int vfio_set_dirty_page_tracking(VFIODevice *vbasedev, bool start)
> -{
> - int ret;
> - VFIOMigration *migration = vbasedev->migration;
> - VFIOContainer *container = vbasedev->group->container;
> - struct vfio_iommu_type1_dirty_bitmap dirty = {
> - .argsz = sizeof(dirty),
> - };
> -
> - if (start) {
> - if (migration->device_state & VFIO_DEVICE_STATE_SAVING) {
> - dirty.flags = VFIO_IOMMU_DIRTY_PAGES_FLAG_START;
> - } else {
> - return -EINVAL;
> - }
> - } else {
> - dirty.flags = VFIO_IOMMU_DIRTY_PAGES_FLAG_STOP;
> - }
> -
> - ret = ioctl(container->fd, VFIO_IOMMU_DIRTY_PAGES, &dirty);
> - if (ret) {
> - error_report("Failed to set dirty tracking flag 0x%x errno: %d",
> - dirty.flags, errno);
> - return -errno;
> - }
> - return ret;
> -}
> -
> static void vfio_migration_cleanup(VFIODevice *vbasedev)
> {
> VFIOMigration *migration = vbasedev->migration;
>
> - vfio_set_dirty_page_tracking(vbasedev, false);
> -
> if (migration->region.mmaps) {
> vfio_region_unmap(&migration->region);
> }
> @@ -469,11 +439,6 @@ static int vfio_save_setup(QEMUFile *f, void *opaque)
> return ret;
> }
>
> - ret = vfio_set_dirty_page_tracking(vbasedev, true);
> - if (ret) {
> - return ret;
> - }
> -
> qemu_put_be64(f, VFIO_MIG_FLAG_END_OF_STATE);
>
> ret = qemu_file_get_error(f);