1. Patchew
  2. QEMU
  3. [PATCH v3 00/10] hw/vfio: IOMMUFD Dirty Tracking
  4. View patch

[PATCH v3 09/10] vfio/migration: Don't block migration device dirty tracking is unsupported

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 09/10] vfio/migration: Don't block migration device dirty tracking is unsupported
Posted by Joao Martins 4 months, 2 weeks ago 
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.

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

diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
index 34d4be2ce1b1..89195928666f 100644
--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -1012,6 +1012,7 @@ void vfio_reset_bytes_transferred(void)
  */
 bool vfio_migration_realize(VFIODevice *vbasedev, Error **errp)
 {
+    HostIOMMUDeviceClass *hiodc = HOST_IOMMU_DEVICE_GET_CLASS(vbasedev->hiod);
     Error *err = NULL;
     int ret;
 
@@ -1036,7 +1037,10 @@ 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->iommufd &&
+         !hiodc->get_cap(vbasedev->hiod,
+                         HOST_IOMMU_DEVICE_CAP_DIRTY_TRACKING, NULL))) {
         if (vbasedev->enable_migration == ON_OFF_AUTO_AUTO) {
             error_setg(&err,
                        "%s: VFIO device doesn't support device dirty tracking",
-- 
2.17.2
Re: [PATCH v3 09/10] vfio/migration: Don't block migration device dirty tracking is unsupported
Posted by Cédric Le Goater 4 months, 2 weeks ago 
On 7/8/24 4:34 PM, 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
> VF dirty page tracking allowing us to minimize the VF requirements for
> migration and thus enabling migration by default for those.
> 
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> ---
>   hw/vfio/migration.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
> index 34d4be2ce1b1..89195928666f 100644
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
> @@ -1012,6 +1012,7 @@ void vfio_reset_bytes_transferred(void)
>    */
>   bool vfio_migration_realize(VFIODevice *vbasedev, Error **errp)
>   {
> +    HostIOMMUDeviceClass *hiodc = HOST_IOMMU_DEVICE_GET_CLASS(vbasedev->hiod);
>       Error *err = NULL;
>       int ret;
>   
> @@ -1036,7 +1037,10 @@ 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->iommufd &&


I don't think we need to check ->iommufd. The class handler below will
return false for the vfio/legacy backend.

Thanks,

C.



> +         !hiodc->get_cap(vbasedev->hiod,
> +                         HOST_IOMMU_DEVICE_CAP_DIRTY_TRACKING, NULL))) {
>           if (vbasedev->enable_migration == ON_OFF_AUTO_AUTO) {
>               error_setg(&err,
>                          "%s: VFIO device doesn't support device dirty tracking",
Re: [PATCH v3 09/10] vfio/migration: Don't block migration device dirty tracking is unsupported
Posted by Joao Martins 4 months, 2 weeks ago 
On 09/07/2024 08:02, Cédric Le Goater wrote:
> On 7/8/24 4:34 PM, 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
>> VF dirty page tracking allowing us to minimize the VF requirements for
>> migration and thus enabling migration by default for those.
>>
>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>> ---
>>   hw/vfio/migration.c | 6 +++++-
>>   1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>> index 34d4be2ce1b1..89195928666f 100644
>> --- a/hw/vfio/migration.c
>> +++ b/hw/vfio/migration.c
>> @@ -1012,6 +1012,7 @@ void vfio_reset_bytes_transferred(void)
>>    */
>>   bool vfio_migration_realize(VFIODevice *vbasedev, Error **errp)
>>   {
>> +    HostIOMMUDeviceClass *hiodc = HOST_IOMMU_DEVICE_GET_CLASS(vbasedev->hiod);
>>       Error *err = NULL;
>>       int ret;
>>   @@ -1036,7 +1037,10 @@ 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->iommufd &&
> 
> 
> I don't think we need to check ->iommufd. The class handler below will
> return false for the vfio/legacy backend.
> 

OK

	Joao
RE: [PATCH v3 09/10] vfio/migration: Don't block migration device dirty tracking is unsupported
Posted by Duan, Zhenzhong 4 months, 2 weeks ago 


>-----Original Message-----
>From: Joao Martins <joao.m.martins@oracle.com>
>Subject: [PATCH v3 09/10] 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.
>
>Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>---
> hw/vfio/migration.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
>diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>index 34d4be2ce1b1..89195928666f 100644
>--- a/hw/vfio/migration.c
>+++ b/hw/vfio/migration.c
>@@ -1012,6 +1012,7 @@ void vfio_reset_bytes_transferred(void)
>  */
> bool vfio_migration_realize(VFIODevice *vbasedev, Error **errp)
> {
>+    HostIOMMUDeviceClass *hiodc =
>HOST_IOMMU_DEVICE_GET_CLASS(vbasedev->hiod);
>     Error *err = NULL;
>     int ret;
>
>@@ -1036,7 +1037,10 @@ 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->iommufd &&
>+         !hiodc->get_cap(vbasedev->hiod,
>+                         HOST_IOMMU_DEVICE_CAP_DIRTY_TRACKING, NULL))) {

What about below, this can avoid a new CAP define.

--- a/hw/vfio/migration.c
+++ b/hw/vfio/migration.c
@@ -1036,7 +1036,7 @@ 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",

Thanks
Zhenzhong

>         if (vbasedev->enable_migration == ON_OFF_AUTO_AUTO) {
>             error_setg(&err,
>                        "%s: VFIO device doesn't support device dirty tracking",
>--
>2.17.2
Re: [PATCH v3 09/10] vfio/migration: Don't block migration device dirty tracking is unsupported
Posted by Joao Martins 4 months, 2 weeks ago 
On 10/07/2024 11:38, Duan, Zhenzhong wrote:
> 
> 
>> -----Original Message-----
>> From: Joao Martins <joao.m.martins@oracle.com>
>> Subject: [PATCH v3 09/10] 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.
>>
>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>> ---
>> hw/vfio/migration.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/hw/vfio/migration.c b/hw/vfio/migration.c
>> index 34d4be2ce1b1..89195928666f 100644
>> --- a/hw/vfio/migration.c
>> +++ b/hw/vfio/migration.c
>> @@ -1012,6 +1012,7 @@ void vfio_reset_bytes_transferred(void)
>>  */
>> bool vfio_migration_realize(VFIODevice *vbasedev, Error **errp)
>> {
>> +    HostIOMMUDeviceClass *hiodc =
>> HOST_IOMMU_DEVICE_GET_CLASS(vbasedev->hiod);
>>     Error *err = NULL;
>>     int ret;
>>
>> @@ -1036,7 +1037,10 @@ 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->iommufd &&
>> +         !hiodc->get_cap(vbasedev->hiod,
>> +                         HOST_IOMMU_DEVICE_CAP_DIRTY_TRACKING, NULL))) {
> 
> What about below, this can avoid a new CAP define.
> 
> --- a/hw/vfio/migration.c
> +++ b/hw/vfio/migration.c
> @@ -1036,7 +1036,7 @@ 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",
> 

From the kernel POV this is supposedly device specific, and the
container::dirty_pages_supported doesn't quite capture a case where the system
is less homogeneous (aka more than one hwpt where one has dirty tracking and the
other doesn't). So asking the HostIOMMUDevice sort of futureproof it (and better
represents the kernel interface). But I don't know of systems like this. And
furthemore mix and match of device dirty tracker with IOMMU dirty tracker isn't
supported in code, so for now I can look at bcontainer::dirty_pages_supported
and I'll remove the CAP addition.

	Joao

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.