>-----Original Message-----
>From: Prasad Pandit <ppan...@redhat.com>
>Subject: Re: [PATCH v1 6/6] intel_iommu: Block migration if cap is updated
>
>On Mon, 4 Mar 2024 at 13:41, Duan, Zhenzhong
><zhenzhong.d...@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.
Yes, iommufd_device_get_info() return ret !=0 means error happen,
we should not update s->cap definitely.
Normally if there are multiple vfio devices backed by different host IOMMUs,
It's possible s->cap updated more than once, e.g., MGAW update: 57->52->48.
>
>* With the above tweaks included:
> Reviewed-by: Prasad Pandit <p...@fedoraproject.org>
Thanks for your review.
BRs.
Zhenzhong
