By default VFIO migration is set to auto, which will support live
migration if the migration capability is set *and* also dirty page
tracking is supported.

For testing purposes one can force enable without dirty page tracking
via enable-migration=on, but that option is generally left for testing
purposes.

So starting with IOMMU dirty tracking it can use to acomodate the lack of
VF dirty page tracking allowing us to minimize the VF requirements for
migration and thus enabling migration by default for those too.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
---
 hw/vfio/migration.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 34d4be2ce1b1..ce3d1b6e9a25 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -1036,7 +1036,8 @@ bool vfio_migration_realize(VFIODevice *vbasedev, Error **errp)
         return !vfio_block_migration(vbasedev, err, errp);
     }
 
-    if (!vbasedev->dirty_pages_supported) {
+    if (!vbasedev->dirty_pages_supported &&
+        !vbasedev->bcontainer->dirty_pages_supported) {
         if (vbasedev->enable_migration == ON_OFF_AUTO_AUTO) {
             error_setg(&err,
                        "%s: VFIO device doesn't support device dirty tracking",
-- 
2.17.2

On 7/12/24 13:47, Joao Martins wrote:
> By default VFIO migration is set to auto, which will support live
> migration if the migration capability is set *and* also dirty page
> tracking is supported.
>
> For testing purposes one can force enable without dirty page tracking
> via enable-migration=on, but that option is generally left for testing
> purposes.
>
> So starting with IOMMU dirty tracking it can use to acomodate the lack of
accomodate

Eric
> VF dirty page tracking allowing us to minimize the VF requirements for
> migration and thus enabling migration by default for those too.
>
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> ---
>  hw/vfio/migration.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> index 34d4be2ce1b1..ce3d1b6e9a25 100644
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
> @@ -1036,7 +1036,8 @@ bool vfio_migration_realize(VFIODevice *vbasedev, Error **errp)
>          return !vfio_block_migration(vbasedev, err, errp);
>      }
>  
> -    if (!vbasedev->dirty_pages_supported) {
> +    if (!vbasedev->dirty_pages_supported &&
> +        !vbasedev->bcontainer->dirty_pages_supported) {
>          if (vbasedev->enable_migration == ON_OFF_AUTO_AUTO) {
>              error_setg(&err,
>                         "%s: VFIO device doesn't support device dirty tracking",

>-----Original Message-----
>From: Joao Martins <joao.m.martins@oracle.com>
>Subject: [PATCH v4 11/12] vfio/migration: Don't block migration device dirty
>tracking is unsupported
>
>By default VFIO migration is set to auto, which will support live
>migration if the migration capability is set *and* also dirty page
>tracking is supported.
>
>For testing purposes one can force enable without dirty page tracking
>via enable-migration=on, but that option is generally left for testing
>purposes.
>
>So starting with IOMMU dirty tracking it can use to acomodate the lack of
>VF dirty page tracking allowing us to minimize the VF requirements for
>migration and thus enabling migration by default for those too.
>
>Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>---
> hw/vfio/migration.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
>diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>index 34d4be2ce1b1..ce3d1b6e9a25 100644
>--- a/hw/vfio/migration.c
>+++ b/hw/vfio/migration.c
>@@ -1036,7 +1036,8 @@ bool vfio_migration_realize(VFIODevice
>*vbasedev, Error **errp)
>         return !vfio_block_migration(vbasedev, err, errp);
>     }
>
>-    if (!vbasedev->dirty_pages_supported) {
>+    if (!vbasedev->dirty_pages_supported &&
>+        !vbasedev->bcontainer->dirty_pages_supported) {
>         if (vbasedev->enable_migration == ON_OFF_AUTO_AUTO) {
>             error_setg(&err,
>                        "%s: VFIO device doesn't support device dirty tracking",

I'm not sure if this message needs to be updated, " VFIO device doesn't support device and IOMMU dirty tracking"

Same for the below:

warn_report("%s: VFIO device doesn't support device dirty tracking"

On 17/07/2024 03:38, Duan, Zhenzhong wrote:
> 
> 
>> -----Original Message-----
>> From: Joao Martins <joao.m.martins@oracle.com>
>> Subject: [PATCH v4 11/12] vfio/migration: Don't block migration device dirty
>> tracking is unsupported
>>
>> By default VFIO migration is set to auto, which will support live
>> migration if the migration capability is set *and* also dirty page
>> tracking is supported.
>>
>> For testing purposes one can force enable without dirty page tracking
>> via enable-migration=on, but that option is generally left for testing
>> purposes.
>>
>> So starting with IOMMU dirty tracking it can use to acomodate the lack of
>> VF dirty page tracking allowing us to minimize the VF requirements for
>> migration and thus enabling migration by default for those too.
>>
>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>> ---
>> hw/vfio/migration.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>> index 34d4be2ce1b1..ce3d1b6e9a25 100644
>> --- a/hw/vfio/migration.c
>> +++ b/hw/vfio/migration.c
>> @@ -1036,7 +1036,8 @@ bool vfio_migration_realize(VFIODevice
>> *vbasedev, Error **errp)
>>         return !vfio_block_migration(vbasedev, err, errp);
>>     }
>>
>> -    if (!vbasedev->dirty_pages_supported) {
>> +    if (!vbasedev->dirty_pages_supported &&
>> +        !vbasedev->bcontainer->dirty_pages_supported) {
>>         if (vbasedev->enable_migration == ON_OFF_AUTO_AUTO) {
>>             error_setg(&err,
>>                        "%s: VFIO device doesn't support device dirty tracking",
> 
> I'm not sure if this message needs to be updated, " VFIO device doesn't support device and IOMMU dirty tracking"
> 
> Same for the below:
> 
> warn_report("%s: VFIO device doesn't support device dirty tracking"


Ah yes, good catch. Additionally I think I should check device hwpt rather than
container::dirty_pages_supported i.e.

if (!vbasedev->dirty_pages_supported &&
    (vbasedev->hwpt && !iommufd_hwpt_dirty_tracking(vbasedev->hwpt)))

This makes sure that migration is blocked with more accuracy

>-----Original Message-----
>From: Joao Martins <joao.m.martins@oracle.com>
>Subject: Re: [PATCH v4 11/12] vfio/migration: Don't block migration device
>dirty tracking is unsupported
>
>On 17/07/2024 03:38, Duan, Zhenzhong wrote:
>>
>>
>>> -----Original Message-----
>>> From: Joao Martins <joao.m.martins@oracle.com>
>>> Subject: [PATCH v4 11/12] vfio/migration: Don't block migration device
>dirty
>>> tracking is unsupported
>>>
>>> By default VFIO migration is set to auto, which will support live
>>> migration if the migration capability is set *and* also dirty page
>>> tracking is supported.
>>>
>>> For testing purposes one can force enable without dirty page tracking
>>> via enable-migration=on, but that option is generally left for testing
>>> purposes.
>>>
>>> So starting with IOMMU dirty tracking it can use to acomodate the lack of
>>> VF dirty page tracking allowing us to minimize the VF requirements for
>>> migration and thus enabling migration by default for those too.
>>>
>>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>>> ---
>>> hw/vfio/migration.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>>> index 34d4be2ce1b1..ce3d1b6e9a25 100644
>>> --- a/hw/vfio/migration.c
>>> +++ b/hw/vfio/migration.c
>>> @@ -1036,7 +1036,8 @@ bool vfio_migration_realize(VFIODevice
>>> *vbasedev, Error **errp)
>>>         return !vfio_block_migration(vbasedev, err, errp);
>>>     }
>>>
>>> -    if (!vbasedev->dirty_pages_supported) {
>>> +    if (!vbasedev->dirty_pages_supported &&
>>> +        !vbasedev->bcontainer->dirty_pages_supported) {
>>>         if (vbasedev->enable_migration == ON_OFF_AUTO_AUTO) {
>>>             error_setg(&err,
>>>                        "%s: VFIO device doesn't support device dirty tracking",
>>
>> I'm not sure if this message needs to be updated, " VFIO device doesn't
>support device and IOMMU dirty tracking"
>>
>> Same for the below:
>>
>> warn_report("%s: VFIO device doesn't support device dirty tracking"
>
>
>Ah yes, good catch. Additionally I think I should check device hwpt rather
>than
>container::dirty_pages_supported i.e.
>
>if (!vbasedev->dirty_pages_supported &&
>    (vbasedev->hwpt && !iommufd_hwpt_dirty_tracking(vbasedev->hwpt)))
>
>This makes sure that migration is blocked with more accuracy

Yes, this is better. Looks bcontainer->dirty_pages_supported is not as accurate as in legacy VFIO days.

Thanks
Zhenzhong

On 18/07/2024 08:20, Duan, Zhenzhong wrote:
> 
> 
>> -----Original Message-----
>> From: Joao Martins <joao.m.martins@oracle.com>
>> Subject: Re: [PATCH v4 11/12] vfio/migration: Don't block migration device
>> dirty tracking is unsupported
>>
>> On 17/07/2024 03:38, Duan, Zhenzhong wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Joao Martins <joao.m.martins@oracle.com>
>>>> Subject: [PATCH v4 11/12] vfio/migration: Don't block migration device
>> dirty
>>>> tracking is unsupported
>>>>
>>>> By default VFIO migration is set to auto, which will support live
>>>> migration if the migration capability is set *and* also dirty page
>>>> tracking is supported.
>>>>
>>>> For testing purposes one can force enable without dirty page tracking
>>>> via enable-migration=on, but that option is generally left for testing
>>>> purposes.
>>>>
>>>> So starting with IOMMU dirty tracking it can use to acomodate the lack of
>>>> VF dirty page tracking allowing us to minimize the VF requirements for
>>>> migration and thus enabling migration by default for those too.
>>>>
>>>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>>>> ---
>>>> hw/vfio/migration.c | 3 ++-
>>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>>>> index 34d4be2ce1b1..ce3d1b6e9a25 100644
>>>> --- a/hw/vfio/migration.c
>>>> +++ b/hw/vfio/migration.c
>>>> @@ -1036,7 +1036,8 @@ bool vfio_migration_realize(VFIODevice
>>>> *vbasedev, Error **errp)
>>>>         return !vfio_block_migration(vbasedev, err, errp);
>>>>     }
>>>>
>>>> -    if (!vbasedev->dirty_pages_supported) {
>>>> +    if (!vbasedev->dirty_pages_supported &&
>>>> +        !vbasedev->bcontainer->dirty_pages_supported) {
>>>>         if (vbasedev->enable_migration == ON_OFF_AUTO_AUTO) {
>>>>             error_setg(&err,
>>>>                        "%s: VFIO device doesn't support device dirty tracking",
>>>
>>> I'm not sure if this message needs to be updated, " VFIO device doesn't
>> support device and IOMMU dirty tracking"
>>>
>>> Same for the below:
>>>
>>> warn_report("%s: VFIO device doesn't support device dirty tracking"
>>
>>
>> Ah yes, good catch. Additionally I think I should check device hwpt rather
>> than
>> container::dirty_pages_supported i.e.
>>
>> if (!vbasedev->dirty_pages_supported &&
>>    (vbasedev->hwpt && !iommufd_hwpt_dirty_tracking(vbasedev->hwpt)))
>>
>> This makes sure that migration is blocked with more accuracy
> 
> Yes, this is better. Looks bcontainer->dirty_pages_supported is not as accurate as in legacy VFIO days.
> 

Heh, That's just because legacy is always marking true (and marking anything as
dirty) regardless of what the hardware does :)

On 17/07/2024 10:20, Joao Martins wrote:
> On 17/07/2024 03:38, Duan, Zhenzhong wrote:
>>
>>
>>> -----Original Message-----
>>> From: Joao Martins <joao.m.martins@oracle.com>
>>> Subject: [PATCH v4 11/12] vfio/migration: Don't block migration device dirty
>>> tracking is unsupported
>>>
>>> By default VFIO migration is set to auto, which will support live
>>> migration if the migration capability is set *and* also dirty page
>>> tracking is supported.
>>>
>>> For testing purposes one can force enable without dirty page tracking
>>> via enable-migration=on, but that option is generally left for testing
>>> purposes.
>>>
>>> So starting with IOMMU dirty tracking it can use to acomodate the lack of
>>> VF dirty page tracking allowing us to minimize the VF requirements for
>>> migration and thus enabling migration by default for those too.
>>>
>>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>>> ---
>>> hw/vfio/migration.c | 3 ++-
>>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>>> index 34d4be2ce1b1..ce3d1b6e9a25 100644
>>> --- a/hw/vfio/migration.c
>>> +++ b/hw/vfio/migration.c
>>> @@ -1036,7 +1036,8 @@ bool vfio_migration_realize(VFIODevice
>>> *vbasedev, Error **errp)
>>>         return !vfio_block_migration(vbasedev, err, errp);
>>>     }
>>>
>>> -    if (!vbasedev->dirty_pages_supported) {
>>> +    if (!vbasedev->dirty_pages_supported &&
>>> +        !vbasedev->bcontainer->dirty_pages_supported) {
>>>         if (vbasedev->enable_migration == ON_OFF_AUTO_AUTO) {
>>>             error_setg(&err,
>>>                        "%s: VFIO device doesn't support device dirty tracking",
>>
>> I'm not sure if this message needs to be updated, " VFIO device doesn't support device and IOMMU dirty tracking"
>>
>> Same for the below:
>>
>> warn_report("%s: VFIO device doesn't support device dirty tracking"
> 
> 
> Ah yes, good catch. Additionally I think I should check device hwpt rather than
> container::dirty_pages_supported i.e.
> 
> if (!vbasedev->dirty_pages_supported &&
>     (vbasedev->hwpt && !iommufd_hwpt_dirty_tracking(vbasedev->hwpt)))
> 
> This makes sure that migration is blocked with more accuracy

I retract this comment as I think it can all be easily detected by not OR-ing
the setting of vbasedev->bcontainer->dirty_pages_supported. I should put a
warn_report_once() there.

	Joao

On 17/07/2024 16:35, Joao Martins wrote:
> On 17/07/2024 10:20, Joao Martins wrote:
>> On 17/07/2024 03:38, Duan, Zhenzhong wrote:
>>>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>>>> index 34d4be2ce1b1..ce3d1b6e9a25 100644
>>>> --- a/hw/vfio/migration.c
>>>> +++ b/hw/vfio/migration.c
>>>> @@ -1036,7 +1036,8 @@ bool vfio_migration_realize(VFIODevice
>>>> *vbasedev, Error **errp)
>>>>         return !vfio_block_migration(vbasedev, err, errp);
>>>>     }
>>>>
>>>> -    if (!vbasedev->dirty_pages_supported) {
>>>> +    if (!vbasedev->dirty_pages_supported &&
>>>> +        !vbasedev->bcontainer->dirty_pages_supported) {
>>>>         if (vbasedev->enable_migration == ON_OFF_AUTO_AUTO) {
>>>>             error_setg(&err,
>>>>                        "%s: VFIO device doesn't support device dirty tracking",
>>>
>>> I'm not sure if this message needs to be updated, " VFIO device doesn't support device and IOMMU dirty tracking"
>>>
>>> Same for the below:
>>>
>>> warn_report("%s: VFIO device doesn't support device dirty tracking"
>>
>>
>> Ah yes, good catch. Additionally I think I should check device hwpt rather than
>> container::dirty_pages_supported i.e.
>>
>> if (!vbasedev->dirty_pages_supported &&
>>     (vbasedev->hwpt && !iommufd_hwpt_dirty_tracking(vbasedev->hwpt)))
>>
>> This makes sure that migration is blocked with more accuracy
> 
> I retract this comment as I think it can all be easily detected by not OR-ing
> the setting of vbasedev->bcontainer->dirty_pages_supported. I should put a
> warn_report_once() there.

Something like this below.

To be clear: this is mostly a safe guard against a theoretic case that we don't
know it exists. For example on x86, this is homogeneous and I suspect server ARM
to be the case too. embedded ARM might be different as there's so many
incantations of it.

@@ -267,6 +282,13 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
     vbasedev->hwpt = hwpt;
     QLIST_INSERT_HEAD(&hwpt->device_list, vbasedev, hwpt_next);
     QLIST_INSERT_HEAD(&container->hwpt_list, hwpt, next);
+
+    if (container->bcontainer.dirty_pages_supported &&
+        !iommufd_hwpt_dirty_tracking(hwpt)) {
+        warn_report("%s: IOMMU dirty tracking not supported\n", vbasedev->name);
+    }
+    container->bcontainer.dirty_pages_supported =
+                              iommufd_hwpt_dirty_tracking(hwpt);
     return true;
 }

On 17/07/2024 17:02, Joao Martins wrote:
> On 17/07/2024 16:35, Joao Martins wrote:
>> On 17/07/2024 10:20, Joao Martins wrote:
>>> On 17/07/2024 03:38, Duan, Zhenzhong wrote:
>>>>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>>>>> index 34d4be2ce1b1..ce3d1b6e9a25 100644
>>>>> --- a/hw/vfio/migration.c
>>>>> +++ b/hw/vfio/migration.c
>>>>> @@ -1036,7 +1036,8 @@ bool vfio_migration_realize(VFIODevice
>>>>> *vbasedev, Error **errp)
>>>>>         return !vfio_block_migration(vbasedev, err, errp);
>>>>>     }
>>>>>
>>>>> -    if (!vbasedev->dirty_pages_supported) {
>>>>> +    if (!vbasedev->dirty_pages_supported &&
>>>>> +        !vbasedev->bcontainer->dirty_pages_supported) {
>>>>>         if (vbasedev->enable_migration == ON_OFF_AUTO_AUTO) {
>>>>>             error_setg(&err,
>>>>>                        "%s: VFIO device doesn't support device dirty tracking",
>>>>
>>>> I'm not sure if this message needs to be updated, " VFIO device doesn't support device and IOMMU dirty tracking"
>>>>
>>>> Same for the below:
>>>>
>>>> warn_report("%s: VFIO device doesn't support device dirty tracking"
>>>
>>>
>>> Ah yes, good catch. Additionally I think I should check device hwpt rather than
>>> container::dirty_pages_supported i.e.
>>>
>>> if (!vbasedev->dirty_pages_supported &&
>>>     (vbasedev->hwpt && !iommufd_hwpt_dirty_tracking(vbasedev->hwpt)))
>>>
>>> This makes sure that migration is blocked with more accuracy
>>
>> I retract this comment as I think it can all be easily detected by not OR-ing
>> the setting of vbasedev->bcontainer->dirty_pages_supported. I should put a
>> warn_report_once() there.
> 
> Something like this below.
> 
> To be clear: this is mostly a safe guard against a theoretic case that we don't
> know it exists. For example on x86, this is homogeneous and I suspect server ARM
> to be the case too. embedded ARM might be different as there's so many
> incantations of it.
> 

Except that it won't work with hotplug :( so the previous snip was actually a
bit better.

> @@ -267,6 +282,13 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
>      vbasedev->hwpt = hwpt;
>      QLIST_INSERT_HEAD(&hwpt->device_list, vbasedev, hwpt_next);
>      QLIST_INSERT_HEAD(&container->hwpt_list, hwpt, next);
> +
> +    if (container->bcontainer.dirty_pages_supported &&
> +        !iommufd_hwpt_dirty_tracking(hwpt)) {
> +        warn_report("%s: IOMMU dirty tracking not supported\n", vbasedev->name);
> +    }
> +    container->bcontainer.dirty_pages_supported =
> +                              iommufd_hwpt_dirty_tracking(hwpt);
>      return true;
>  }
> 
>