[PATCH v3 10/10] vfio/common: Allow disabling device dirty page tracking

Joao Martins posted 10 patches 4 months, 2 weeks ago
Maintainers: Yi Liu <yi.l.liu@intel.com>, Eric Auger <eric.auger@redhat.com>, Zhenzhong Duan <zhenzhong.duan@intel.com>, Alex Williamson <alex.williamson@redhat.com>, "Cédric Le Goater" <clg@redhat.com>
[PATCH v3 10/10] vfio/common: Allow disabling device dirty page tracking
Posted by Joao Martins 4 months, 2 weeks ago
The property 'x-pre-copy-dirty-page-tracking' allows disabling the whole
tracking of VF pre-copy phase of dirty page tracking, though it means
that it will only be used at the start of the switchover phase.

Add an option that disables the VF dirty page tracking, and fall
back into container-based dirty page tracking. This also allows to
use IOMMU dirty tracking even on VFs with their own dirty
tracker scheme.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
---
 include/hw/vfio/vfio-common.h | 1 +
 hw/vfio/common.c              | 3 +++
 hw/vfio/migration.c           | 3 ++-
 hw/vfio/pci.c                 | 3 +++
 4 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 7ce925cfab19..9db3fd31cfae 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -137,6 +137,7 @@ typedef struct VFIODevice {
     VFIOMigration *migration;
     Error *migration_blocker;
     OnOffAuto pre_copy_dirty_page_tracking;
+    OnOffAuto device_dirty_page_tracking;
     bool dirty_pages_supported;
     bool dirty_tracking;
     HostIOMMUDevice *hiod;
diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 7cdb969fd396..eaa33d9ee037 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -199,6 +199,9 @@ bool vfio_devices_all_device_dirty_tracking(const VFIOContainerBase *bcontainer)
     VFIODevice *vbasedev;
 
     QLIST_FOREACH(vbasedev, &bcontainer->device_list, container_next) {
+        if (vbasedev->device_dirty_page_tracking == ON_OFF_AUTO_OFF) {
+            return false;
+        }
         if (!vbasedev->dirty_pages_supported) {
             return false;
         }
diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 89195928666f..35d67332db20 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -1037,7 +1037,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->device_dirty_page_tracking == ON_OFF_AUTO_OFF) &&
         (vbasedev->iommufd &&
          !hiodc->get_cap(vbasedev->hiod,
                          HOST_IOMMU_DEVICE_CAP_DIRTY_TRACKING, NULL))) {
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index e03d9f3ba546..22819b2036b3 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3362,6 +3362,9 @@ static Property vfio_pci_dev_properties[] = {
     DEFINE_PROP_ON_OFF_AUTO("x-pre-copy-dirty-page-tracking", VFIOPCIDevice,
                             vbasedev.pre_copy_dirty_page_tracking,
                             ON_OFF_AUTO_ON),
+    DEFINE_PROP_ON_OFF_AUTO("x-device-dirty-page-tracking", VFIOPCIDevice,
+                            vbasedev.device_dirty_page_tracking,
+                            ON_OFF_AUTO_ON),
     DEFINE_PROP_ON_OFF_AUTO("display", VFIOPCIDevice,
                             display, ON_OFF_AUTO_OFF),
     DEFINE_PROP_UINT32("xres", VFIOPCIDevice, display_xres, 0),
-- 
2.17.2
RE: [PATCH v3 10/10] vfio/common: Allow disabling device dirty page tracking
Posted by Duan, Zhenzhong 4 months, 2 weeks ago

>-----Original Message-----
>From: Joao Martins <joao.m.martins@oracle.com>
>Subject: [PATCH v3 10/10] vfio/common: Allow disabling device dirty page
>tracking
>
>The property 'x-pre-copy-dirty-page-tracking' allows disabling the whole
>tracking of VF pre-copy phase of dirty page tracking, though it means
>that it will only be used at the start of the switchover phase.
>
>Add an option that disables the VF dirty page tracking, and fall
>back into container-based dirty page tracking. This also allows to
>use IOMMU dirty tracking even on VFs with their own dirty
>tracker scheme.
>
>Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>---
> include/hw/vfio/vfio-common.h | 1 +
> hw/vfio/common.c              | 3 +++
> hw/vfio/migration.c           | 3 ++-
> hw/vfio/pci.c                 | 3 +++
> 4 files changed, 9 insertions(+), 1 deletion(-)
>
>diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-
>common.h
>index 7ce925cfab19..9db3fd31cfae 100644
>--- a/include/hw/vfio/vfio-common.h
>+++ b/include/hw/vfio/vfio-common.h
>@@ -137,6 +137,7 @@ typedef struct VFIODevice {
>     VFIOMigration *migration;
>     Error *migration_blocker;
>     OnOffAuto pre_copy_dirty_page_tracking;
>+    OnOffAuto device_dirty_page_tracking;
>     bool dirty_pages_supported;
>     bool dirty_tracking;
>     HostIOMMUDevice *hiod;
>diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>index 7cdb969fd396..eaa33d9ee037 100644
>--- a/hw/vfio/common.c
>+++ b/hw/vfio/common.c
>@@ -199,6 +199,9 @@ bool vfio_devices_all_device_dirty_tracking(const
>VFIOContainerBase *bcontainer)
>     VFIODevice *vbasedev;
>
>     QLIST_FOREACH(vbasedev, &bcontainer->device_list, container_next) {
>+        if (vbasedev->device_dirty_page_tracking == ON_OFF_AUTO_OFF) {
>+            return false;

Maybe we can initialize vbasedev->dirty_pages_supported to false by checking vbasedev->device_dirty_page_tracking == ON_OFF_AUTO_OFF?
This way we can avoid extra check.

>+        }
>         if (!vbasedev->dirty_pages_supported) {
>             return false;
>         }
>diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>index 89195928666f..35d67332db20 100644
>--- a/hw/vfio/migration.c
>+++ b/hw/vfio/migration.c
>@@ -1037,7 +1037,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->device_dirty_page_tracking == ON_OFF_AUTO_OFF) &&
>         (vbasedev->iommufd &&
>          !hiodc->get_cap(vbasedev->hiod,
>                          HOST_IOMMU_DEVICE_CAP_DIRTY_TRACKING, NULL))) {
>diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>index e03d9f3ba546..22819b2036b3 100644
>--- a/hw/vfio/pci.c
>+++ b/hw/vfio/pci.c
>@@ -3362,6 +3362,9 @@ static Property vfio_pci_dev_properties[] = {
>     DEFINE_PROP_ON_OFF_AUTO("x-pre-copy-dirty-page-tracking",
>VFIOPCIDevice,
>                             vbasedev.pre_copy_dirty_page_tracking,
>                             ON_OFF_AUTO_ON),
>+    DEFINE_PROP_ON_OFF_AUTO("x-device-dirty-page-tracking",
>VFIOPCIDevice,
>+                            vbasedev.device_dirty_page_tracking,
>+                            ON_OFF_AUTO_ON),
>     DEFINE_PROP_ON_OFF_AUTO("display", VFIOPCIDevice,
>                             display, ON_OFF_AUTO_OFF),
>     DEFINE_PROP_UINT32("xres", VFIOPCIDevice, display_xres, 0),
>--
>2.17.2
Re: [PATCH v3 10/10] vfio/common: Allow disabling device dirty page tracking
Posted by Joao Martins 4 months, 2 weeks ago
On 10/07/2024 11:42, Duan, Zhenzhong wrote:
> 
> 
>> -----Original Message-----
>> From: Joao Martins <joao.m.martins@oracle.com>
>> Subject: [PATCH v3 10/10] vfio/common: Allow disabling device dirty page
>> tracking
>>
>> The property 'x-pre-copy-dirty-page-tracking' allows disabling the whole
>> tracking of VF pre-copy phase of dirty page tracking, though it means
>> that it will only be used at the start of the switchover phase.
>>
>> Add an option that disables the VF dirty page tracking, and fall
>> back into container-based dirty page tracking. This also allows to
>> use IOMMU dirty tracking even on VFs with their own dirty
>> tracker scheme.
>>
>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>> ---
>> include/hw/vfio/vfio-common.h | 1 +
>> hw/vfio/common.c              | 3 +++
>> hw/vfio/migration.c           | 3 ++-
>> hw/vfio/pci.c                 | 3 +++
>> 4 files changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-
>> common.h
>> index 7ce925cfab19..9db3fd31cfae 100644
>> --- a/include/hw/vfio/vfio-common.h
>> +++ b/include/hw/vfio/vfio-common.h
>> @@ -137,6 +137,7 @@ typedef struct VFIODevice {
>>     VFIOMigration *migration;
>>     Error *migration_blocker;
>>     OnOffAuto pre_copy_dirty_page_tracking;
>> +    OnOffAuto device_dirty_page_tracking;
>>     bool dirty_pages_supported;
>>     bool dirty_tracking;
>>     HostIOMMUDevice *hiod;
>> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>> index 7cdb969fd396..eaa33d9ee037 100644
>> --- a/hw/vfio/common.c
>> +++ b/hw/vfio/common.c
>> @@ -199,6 +199,9 @@ bool vfio_devices_all_device_dirty_tracking(const
>> VFIOContainerBase *bcontainer)
>>     VFIODevice *vbasedev;
>>
>>     QLIST_FOREACH(vbasedev, &bcontainer->device_list, container_next) {
>> +        if (vbasedev->device_dirty_page_tracking == ON_OFF_AUTO_OFF) {
>> +            return false;
> 
> Maybe we can initialize vbasedev->dirty_pages_supported to false by checking vbasedev->device_dirty_page_tracking == ON_OFF_AUTO_OFF?
> This way we can avoid extra check.
> 

Hmm, I guess it's a matter of style i.e. the device may support it
(vbasedev::dirty_pages_supported), but user disabled
(vbasedev::device_dirty_page_tracking). It's true that it simplifies, but feels
somewhat misleading?

>> +        }
>>         if (!vbasedev->dirty_pages_supported) {
>>             return false;
>>         }
>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>> index 89195928666f..35d67332db20 100644
>> --- a/hw/vfio/migration.c
>> +++ b/hw/vfio/migration.c
>> @@ -1037,7 +1037,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->device_dirty_page_tracking == ON_OFF_AUTO_OFF) &&
>>         (vbasedev->iommufd &&
>>          !hiodc->get_cap(vbasedev->hiod,
>>                          HOST_IOMMU_DEVICE_CAP_DIRTY_TRACKING, NULL))) {
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>> index e03d9f3ba546..22819b2036b3 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
>> @@ -3362,6 +3362,9 @@ static Property vfio_pci_dev_properties[] = {
>>     DEFINE_PROP_ON_OFF_AUTO("x-pre-copy-dirty-page-tracking",
>> VFIOPCIDevice,
>>                             vbasedev.pre_copy_dirty_page_tracking,
>>                             ON_OFF_AUTO_ON),
>> +    DEFINE_PROP_ON_OFF_AUTO("x-device-dirty-page-tracking",
>> VFIOPCIDevice,
>> +                            vbasedev.device_dirty_page_tracking,
>> +                            ON_OFF_AUTO_ON),
>>     DEFINE_PROP_ON_OFF_AUTO("display", VFIOPCIDevice,
>>                             display, ON_OFF_AUTO_OFF),
>>     DEFINE_PROP_UINT32("xres", VFIOPCIDevice, display_xres, 0),
>> --
>> 2.17.2
>