Re: [PATCH QEMU HCI-8.1 v3 27/28] VZ: vfio/region: preserve BAR mappings across cpr-exec #VSTOR-103860

[no-reply@sashiko.virtuozzo.com]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] vfio: vfio_device_attach() fails during migration/snapshot due to new blocker
--

commit c60b9a0f169a17999aea7d76e3213dbee63fd377
Author: Andrey Drobyshev <andrey.drobyshev@virtuozzo.com>

VZ: vfio/region: preserve BAR mappings across cpr-exec #VSTOR-103860

This patch saves each VFIO BAR mapping in the CPR ramblock list. After
cpr-exec, new QEMU reuses the saved mapping instead of calling mmap()
again. It also adds a blocker so cpr-transfer is refused for VFIO devices.

> diff --git a/hw/vfio/device.c b/hw/vfio/device.c
> --- a/hw/vfio/device.c
> +++ b/hw/vfio/device.c

[ ... ]

> @@ -464,12 +464,32 @@ bool vfio_device_attach(char *name, VFIODevice *vbasedev,
>                               TYPE_VFIO_IOMMU_IOMMUFD :
>                               TYPE_VFIO_IOMMU_LEGACY;
>  
> -    return vfio_device_attach_by_iommu_type(iommu_type, name, vbasedev,
> -                                            as, errp);
> +    /*
> +     * Only cpr-exec keeps the address space and so the region mappings,
> +     * see vfio_region_mmap().  A cpr-transfer target would have to map the
> +     * regions anew, which some drivers (NVIDIA vGPU) refuse.
> +     */
> +    error_setg(&vbasedev->cpr.mmap_blocker,
> +               "cpr-transfer is not supported with VFIO devices; "
> +               "use the cpr-exec mode");
> +    if (migrate_add_blocker_modes(&vbasedev->cpr.mmap_blocker, errp,
> +                                  MIG_MODE_CPR_TRANSFER, -1)) {
> +        return false;
> +    }

[Severity: Low]
Can this make vfio_device_attach() fail while a migration or savevm is
running? migrate_add_blocker_modes() calls is_busy() regardless of which
modes were requested:

migration/migration.c:is_busy() {
    if (runstate_check(RUN_STATE_SAVE_VM) || migration_is_running()) {
        error_propagate_prepend(errp, *reasonp, ...);
        *reasonp = NULL;
        return true;
    }
}

migrate_add_blocker_modes() then returns -EBUSY, and vfio_device_attach()
returns false. So hotplugging a VFIO device during a migration or savevm
would now be refused, even though the new blocker only applies to
cpr-transfer. Before this patch, attach did not depend on migration state
for devices that needed no blocker.

Is that behaviour intended for this blocker?

> +
> +    if (!vfio_device_attach_by_iommu_type(iommu_type, name, vbasedev,
> +                                          as, errp)) {
> +        migrate_del_blocker(&vbasedev->cpr.mmap_blocker);
> +        return false;
> +    }
> +
> +    return true;
>  }

[ ... ]

-- 
Sashiko AI review · https://sashiko.vzint.dev/#/patchset/20261016-vfio_v1-v3-0-47487cb5a8e5@virtuozzo.com?part=27