When there is VFIO device and vIOMMU cap/ecap is updated based on host
IOMMU cap/ecap, migration should be blocked.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 hw/i386/intel_iommu.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
index 72cc8b2c71..7f9ff653b2 100644
--- a/hw/i386/intel_iommu.c
+++ b/hw/i386/intel_iommu.c
@@ -39,6 +39,7 @@
 #include "hw/i386/apic_internal.h"
 #include "kvm/kvm_i386.h"
 #include "migration/vmstate.h"
+#include "migration/blocker.h"
 #include "trace.h"
 
 #define S_AW_BITS (VTD_MGAW_FROM_CAP(s->cap) + 1)
@@ -3829,6 +3830,8 @@ static int vtd_check_legacy_hdev(IntelIOMMUState *s,
     return 0;
 }
 
+static Error *vtd_mig_blocker;
+
 static int vtd_check_iommufd_hdev(IntelIOMMUState *s,
                                   IOMMUFDDevice *idev,
                                   Error **errp)
@@ -3860,8 +3863,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;
 }
 
 static int vtd_check_hdev(IntelIOMMUState *s, VTDHostIOMMUDevice *vtd_hdev,
-- 
2.34.1

On 01/02/2024 07:28, Zhenzhong Duan wrote:
> When there is VFIO device and vIOMMU cap/ecap is updated based on host
> IOMMU cap/ecap, migration should be blocked.
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

Is this really needed considering migration with vIOMMU is already blocked anyways?

> ---
>  hw/i386/intel_iommu.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
> index 72cc8b2c71..7f9ff653b2 100644
> --- a/hw/i386/intel_iommu.c
> +++ b/hw/i386/intel_iommu.c
> @@ -39,6 +39,7 @@
>  #include "hw/i386/apic_internal.h"
>  #include "kvm/kvm_i386.h"
>  #include "migration/vmstate.h"
> +#include "migration/blocker.h"
>  #include "trace.h"
>  
>  #define S_AW_BITS (VTD_MGAW_FROM_CAP(s->cap) + 1)
> @@ -3829,6 +3830,8 @@ static int vtd_check_legacy_hdev(IntelIOMMUState *s,
>      return 0;
>  }
>  
> +static Error *vtd_mig_blocker;
> +
>  static int vtd_check_iommufd_hdev(IntelIOMMUState *s,
>                                    IOMMUFDDevice *idev,
>                                    Error **errp)
> @@ -3860,8 +3863,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;
>  }
>  
>  static int vtd_check_hdev(IntelIOMMUState *s, VTDHostIOMMUDevice *vtd_hdev,

>-----Original Message-----
>From: Joao Martins <joao.m.martins@oracle.com>
>Subject: Re: [PATCH rfcv2 18/18] intel_iommu: Block migration if cap is
>updated
>
>On 01/02/2024 07:28, Zhenzhong Duan wrote:
>> When there is VFIO device and vIOMMU cap/ecap is updated based on
>host
>> IOMMU cap/ecap, migration should be blocked.
>>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>
>Is this really needed considering migration with vIOMMU is already blocked
>anyways?

VFIO device can be hot unplugged, then blocker due to vIOMMU is removed,
but we still need a blocker for cap/ecap update.

Thanks
Zhenzhong

>
>> ---
>>  hw/i386/intel_iommu.c | 16 ++++++++++++++--
>>  1 file changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/i386/intel_iommu.c b/hw/i386/intel_iommu.c
>> index 72cc8b2c71..7f9ff653b2 100644
>> --- a/hw/i386/intel_iommu.c
>> +++ b/hw/i386/intel_iommu.c
>> @@ -39,6 +39,7 @@
>>  #include "hw/i386/apic_internal.h"
>>  #include "kvm/kvm_i386.h"
>>  #include "migration/vmstate.h"
>> +#include "migration/blocker.h"
>>  #include "trace.h"
>>
>>  #define S_AW_BITS (VTD_MGAW_FROM_CAP(s->cap) + 1)
>> @@ -3829,6 +3830,8 @@ static int
>vtd_check_legacy_hdev(IntelIOMMUState *s,
>>      return 0;
>>  }
>>
>> +static Error *vtd_mig_blocker;
>> +
>>  static int vtd_check_iommufd_hdev(IntelIOMMUState *s,
>>                                    IOMMUFDDevice *idev,
>>                                    Error **errp)
>> @@ -3860,8 +3863,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;
>>  }
>>
>>  static int vtd_check_hdev(IntelIOMMUState *s, VTDHostIOMMUDevice
>*vtd_hdev,

On 27/02/2024 02:41, Duan, Zhenzhong wrote:
> 
> 
>> -----Original Message-----
>> From: Joao Martins <joao.m.martins@oracle.com>
>> Subject: Re: [PATCH rfcv2 18/18] intel_iommu: Block migration if cap is
>> updated
>>
>> On 01/02/2024 07:28, Zhenzhong Duan wrote:
>>> When there is VFIO device and vIOMMU cap/ecap is updated based on
>> host
>>> IOMMU cap/ecap, migration should be blocked.
>>>
>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>
>> Is this really needed considering migration with vIOMMU is already blocked
>> anyways?
> 
> VFIO device can be hot unplugged, then blocker due to vIOMMU is removed,
> but we still need a blocker for cap/ecap update.
> 

Right which then the blocker gets re-added after you add one VFIO device. The
commit message refers xplicitly VFIO device, why would you care about blocking
migration on vIOMMU without vfio devices present? Maybe there's another reason
but that the commit messages doesn't cover? like guest MGAW being bigger than
host MGAW or something like that?

	Joao

>-----Original Message-----
>From: Joao Martins <joao.m.martins@oracle.com>
>Subject: Re: [PATCH rfcv2 18/18] intel_iommu: Block migration if cap is
>updated
>
>On 27/02/2024 02:41, Duan, Zhenzhong wrote:
>>
>>
>>> -----Original Message-----
>>> From: Joao Martins <joao.m.martins@oracle.com>
>>> Subject: Re: [PATCH rfcv2 18/18] intel_iommu: Block migration if cap is
>>> updated
>>>
>>> On 01/02/2024 07:28, Zhenzhong Duan wrote:
>>>> When there is VFIO device and vIOMMU cap/ecap is updated based on
>>> host
>>>> IOMMU cap/ecap, migration should be blocked.
>>>>
>>>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>>>
>>> Is this really needed considering migration with vIOMMU is already
>blocked
>>> anyways?
>>
>> VFIO device can be hot unplugged, then blocker due to vIOMMU is
>removed,
>> but we still need a blocker for cap/ecap update.
>>
>
>Right which then the blocker gets re-added after you add one VFIO device.
>The
>commit message refers xplicitly VFIO device, why would you care about
>blocking
>migration on vIOMMU without vfio devices present? Maybe there's another
>reason
>but that the commit messages doesn't cover? like guest MGAW being bigger
>than
>host MGAW or something like that?

If qemu starts with cold plugged vfio device, that vfio device may update cap/ecap.
Even if that vfio device is unplugged at runtime, the changed cap/ecap is kept.
In this case source and dest will have incompatible cap/ecap config.
So I block migration if there is cap/ecap update on source side.

This patch is to deal with the case that there is cold plugged vfio device which is
unplugged at runtime and then migration happen.

Thanks
Zhenzhong