Re: [PATCH v1 6/6] intel_iommu: Block migration if cap is updated

[Prasad Pandit]

On Mon, 4 Mar 2024 at 13:41, Duan, Zhenzhong <zhenzhong.duan@intel.com> wrote:
> This is to deal with a special case when cold plugged vfio device is unplugged
> at runtime, then migration triggers.
>
> When qemu on source side starts with cold plugged vfio device, vIOMMU

qemu -> QEMU

> capability/extended capability registers(cap/ecap) can be updated based
> on host cap/ecap, then vIOMMU cap/ecap is frozen after machine creation
> done, vIOMMU cap/ecap isn’t restored to default after vfio device is 
> unplugged.
> In this case source and dest(default cap/ecap) will have incompatible cap/ecap
> value. So migration is blocked if there is cap/ecap update on source side.
>
> If vfio device isn't unplugged at runtime, vfio device's own vIOMMU migration 
> blocker
> is redundant with blocker here, but that's harmless.
>
> If vfio devices are all hot plugged after qemu on source side starts, then 
> vIOMMU
> cap/ecap is frozen with the default value, we don't make a blocker in this 
> case.
>

Nice +1

> >> @@ -3861,8 +3864,17 @@ static int
> >vtd_check_iommufd_hdev(IntelIOMMUState *s,
> >>          tmp_cap |= VTD_CAP_MGAW(host_mgaw + 1);
> >>      }
> >>
> >> -    s->cap = tmp_cap;
> >> -    return 0;
> >> +    if (s->cap != tmp_cap) {
> >> +        if (vtd_mig_blocker == NULL) {
> >> +            error_setg(&vtd_mig_blocker,
> >> +                       "cap/ecap update from host IOMMU block migration");
> >> +            ret = migrate_add_blocker(&vtd_mig_blocker, errp);
> >> +        }
> >> +        if (!ret) {
> >> +            s->cap = tmp_cap;
> >> +        }
> >> +    }
> >> +    return ret;
>
> vtd_mig_blocker != NULL means cap is already updated once at least.
> In this case, ret is return value 0 from iommufd_device_get_info().
>
>     ret = iommufd_device_get_info(idev, &type, sizeof(vtd), &vtd, errp);
>     if (ret) {
>         return ret;
>     }
>
> Then cap is updated with host cap value tmp_cap. This update only happen
> before machine creation done.

* After iommufd_device_get_info() ret is != 0. So s->cap won't be
updated then. Hope that is intended.

* With the above tweaks included:
    Reviewed-by: Prasad Pandit <pjp@fedoraproject.org>

Thank you.
---
  - Prasad