1. Patchew
  2. QEMU
  3. [PATCH v4 00/12] hw/iommufd: IOMMUFD Dirty Tracking
  4. View patch

[PATCH v4 08/12] vfio/iommufd: Probe and request hwpt dirty tracking capability

Joao Martins posted 12 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>
There is a newer version of this series
[PATCH v4 08/12] vfio/iommufd: Probe and request hwpt dirty tracking capability
Posted by Joao Martins 4 months, 2 weeks ago 
Probe hardware dirty tracking support by querying device hw capabilities via
IOMMUFD_GET_HW_INFO.

In preparation to using the dirty tracking UAPI, request dirty tracking in the
HWPT flags when the IOMMU supports dirty tracking.

The auto domain logic allows different IOMMU domains to be created when DMA
dirty tracking is not desired (and VF can provide it) while others doesn't have
it and want the IOMMU capability. This is not used in this way here given how
VFIODevice migration capability checking takes place *after* the device
attachment.

Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
---
 include/hw/vfio/vfio-common.h |  1 +
 hw/vfio/iommufd.c             | 12 ++++++++++++
 2 files changed, 13 insertions(+)

diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 2dd468ce3c02..760f31d84ac8 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -97,6 +97,7 @@ typedef struct IOMMUFDBackend IOMMUFDBackend;
 
 typedef struct VFIOIOASHwpt {
     uint32_t hwpt_id;
+    uint32_t hwpt_flags;
     QLIST_HEAD(, VFIODevice) device_list;
     QLIST_ENTRY(VFIOIOASHwpt) next;
 } VFIOIOASHwpt;
diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index d34dc88231ec..edc8f97d8f3d 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -246,6 +246,15 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
         }
     }
 
+    /*
+     * This is quite early and VFIODevice isn't yet fully initialized,
+     * thus rely on IOMMU hardware capabilities as to whether IOMMU dirty
+     * tracking is going to be needed.
+     */
+    if (vbasedev->hiod->caps.hw_caps & IOMMU_HW_CAP_DIRTY_TRACKING) {
+        flags = IOMMU_HWPT_ALLOC_DIRTY_TRACKING;
+    }
+
     if (!iommufd_backend_alloc_hwpt(iommufd, vbasedev->devid,
                                     container->ioas_id, flags,
                                     IOMMU_HWPT_DATA_NONE, 0, NULL,
@@ -255,6 +264,7 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
 
     hwpt = g_malloc0(sizeof(*hwpt));
     hwpt->hwpt_id = hwpt_id;
+    hwpt->hwpt_flags = flags;
     QLIST_INIT(&hwpt->device_list);
 
     ret = iommufd_cdev_attach_ioas_hwpt(vbasedev, hwpt->hwpt_id, errp);
@@ -267,6 +277,8 @@ 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);
+    container->bcontainer.dirty_pages_supported |=
+                              (flags & IOMMU_HWPT_ALLOC_DIRTY_TRACKING);
     return true;
 }
 
-- 
2.17.2
Re: [PATCH v4 08/12] vfio/iommufd: Probe and request hwpt dirty tracking capability
Posted by Eric Auger 4 months, 1 week ago 
Hi Joao,

On 7/12/24 13:47, Joao Martins wrote:
> Probe hardware dirty tracking support by querying device hw capabilities via
> IOMMUFD_GET_HW_INFO.
this is not what the patch brings. GET_HW_INFO is always in place.
>
> In preparation to using the dirty tracking UAPI, request dirty tracking in the
> HWPT flags when the IOMMU supports dirty tracking.
this is what the patch brings.
>
> The auto domain logic allows different IOMMU domains to be created when DMA
> dirty tracking is not desired (and VF can provide it) while others doesn't have
don't
> it and want the IOMMU capability. This is not used in this way here given how
> VFIODevice migration capability checking takes place *after* the device
> attachment.
Id on't understand the above sentence

Eric
>
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> ---
>  include/hw/vfio/vfio-common.h |  1 +
>  hw/vfio/iommufd.c             | 12 ++++++++++++
>  2 files changed, 13 insertions(+)
>
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index 2dd468ce3c02..760f31d84ac8 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -97,6 +97,7 @@ typedef struct IOMMUFDBackend IOMMUFDBackend;
>  
>  typedef struct VFIOIOASHwpt {
>      uint32_t hwpt_id;
> +    uint32_t hwpt_flags;
>      QLIST_HEAD(, VFIODevice) device_list;
>      QLIST_ENTRY(VFIOIOASHwpt) next;
>  } VFIOIOASHwpt;
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index d34dc88231ec..edc8f97d8f3d 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -246,6 +246,15 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
>          }
>      }
>  
> +    /*
> +     * This is quite early and VFIODevice isn't yet fully initialized,
so what's the problem exactly with the above?
> +     * thus rely on IOMMU hardware capabilities as to whether IOMMU dirty
> +     * tracking is going to be needed.
> +     */
> +    if (vbasedev->hiod->caps.hw_caps & IOMMU_HW_CAP_DIRTY_TRACKING) {
> +        flags = IOMMU_HWPT_ALLOC_DIRTY_TRACKING;
> +    }
> +
>      if (!iommufd_backend_alloc_hwpt(iommufd, vbasedev->devid,
>                                      container->ioas_id, flags,
>                                      IOMMU_HWPT_DATA_NONE, 0, NULL,
> @@ -255,6 +264,7 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
>  
>      hwpt = g_malloc0(sizeof(*hwpt));
>      hwpt->hwpt_id = hwpt_id;
> +    hwpt->hwpt_flags = flags;
>      QLIST_INIT(&hwpt->device_list);
>  
>      ret = iommufd_cdev_attach_ioas_hwpt(vbasedev, hwpt->hwpt_id, errp);
> @@ -267,6 +277,8 @@ 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);
> +    container->bcontainer.dirty_pages_supported |=
> +                              (flags & IOMMU_HWPT_ALLOC_DIRTY_TRACKING);
>      return true;
>  }
>
Re: [PATCH v4 08/12] vfio/iommufd: Probe and request hwpt dirty tracking capability
Posted by Joao Martins 4 months, 1 week ago 
On 17/07/2024 13:27, Eric Auger wrote:
> Hi Joao,
> 
> On 7/12/24 13:47, Joao Martins wrote:
>> Probe hardware dirty tracking support by querying device hw capabilities via
>> IOMMUFD_GET_HW_INFO.
> this is not what the patch brings. GET_HW_INFO is always in place.

Yes. This is my mistake in squashing things as there was some shuffling going
around on how we do GET_HW_INFO. and didn't adjust the right hand of this sentence.

I'll rephrase it.

>>
>> In preparation to using the dirty tracking UAPI, request dirty tracking in the
>> HWPT flags when the IOMMU supports dirty tracking.
> this is what the patch brings.

Right.

>>
>> The auto domain logic allows different IOMMU domains to be created when DMA
>> dirty tracking is not desired (and VF can provide it) while others doesn't have
> don't

Right

>> it and want the IOMMU capability. This is not used in this way here given how
>> VFIODevice migration capability checking takes place *after* the device
>> attachment.
> Id on't understand the above sentence
> 

The whole paragraph is meant to emphasize that we don't know if VF dirty
tracking is supported because VFIODevice migration state hasn't been probed
*yet*. And so we can't pick VF dirty tracking vs IOMMU dirty tracking at this
stage when using IOMMU_HWPT_ALLOC_DIRTY_TRACKING flag and hence we always use it
if IOMMU hw supports it even if later on VFIOMigration decides to use VF dirty
tracking always instead.

> Eric
>>
>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>> ---
>>  include/hw/vfio/vfio-common.h |  1 +
>>  hw/vfio/iommufd.c             | 12 ++++++++++++
>>  2 files changed, 13 insertions(+)
>>
>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
>> index 2dd468ce3c02..760f31d84ac8 100644
>> --- a/include/hw/vfio/vfio-common.h
>> +++ b/include/hw/vfio/vfio-common.h
>> @@ -97,6 +97,7 @@ typedef struct IOMMUFDBackend IOMMUFDBackend;
>>  
>>  typedef struct VFIOIOASHwpt {
>>      uint32_t hwpt_id;
>> +    uint32_t hwpt_flags;
>>      QLIST_HEAD(, VFIODevice) device_list;
>>      QLIST_ENTRY(VFIOIOASHwpt) next;
>>  } VFIOIOASHwpt;
>> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>> index d34dc88231ec..edc8f97d8f3d 100644
>> --- a/hw/vfio/iommufd.c
>> +++ b/hw/vfio/iommufd.c
>> @@ -246,6 +246,15 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
>>          }
>>      }
>>  
>> +    /*
>> +     * This is quite early and VFIODevice isn't yet fully initialized,
> so what's the problem exactly with the above?

I should really say 'VFIO Migration state' here (see previous comment)

>> +     * thus rely on IOMMU hardware capabilities as to whether IOMMU dirty
>> +     * tracking is going to be needed.
>> +     */
>> +    if (vbasedev->hiod->caps.hw_caps & IOMMU_HW_CAP_DIRTY_TRACKING) {
>> +        flags = IOMMU_HWPT_ALLOC_DIRTY_TRACKING;
>> +    }
>> +
>>      if (!iommufd_backend_alloc_hwpt(iommufd, vbasedev->devid,
>>                                      container->ioas_id, flags,
>>                                      IOMMU_HWPT_DATA_NONE, 0, NULL,
>> @@ -255,6 +264,7 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
>>  
>>      hwpt = g_malloc0(sizeof(*hwpt));
>>      hwpt->hwpt_id = hwpt_id;
>> +    hwpt->hwpt_flags = flags;
>>      QLIST_INIT(&hwpt->device_list);
>>  
>>      ret = iommufd_cdev_attach_ioas_hwpt(vbasedev, hwpt->hwpt_id, errp);
>> @@ -267,6 +277,8 @@ 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);
>> +    container->bcontainer.dirty_pages_supported |=
>> +                              (flags & IOMMU_HWPT_ALLOC_DIRTY_TRACKING);
>>      return true;
>>  }
>>  
>
Re: [PATCH v4 08/12] vfio/iommufd: Probe and request hwpt dirty tracking capability
Posted by Eric Auger 4 months, 1 week ago 

On 7/17/24 14:38, Joao Martins wrote:
> On 17/07/2024 13:27, Eric Auger wrote:
>> Hi Joao,
>>
>> On 7/12/24 13:47, Joao Martins wrote:
>>> Probe hardware dirty tracking support by querying device hw capabilities via
>>> IOMMUFD_GET_HW_INFO.
>> this is not what the patch brings. GET_HW_INFO is always in place.
> Yes. This is my mistake in squashing things as there was some shuffling going
> around on how we do GET_HW_INFO. and didn't adjust the right hand of this sentence.
>
> I'll rephrase it.
>
>>> In preparation to using the dirty tracking UAPI, request dirty tracking in the
>>> HWPT flags when the IOMMU supports dirty tracking.
>> this is what the patch brings.
> Right.
>
>>> The auto domain logic allows different IOMMU domains to be created when DMA
>>> dirty tracking is not desired (and VF can provide it) while others doesn't have
>> don't
> Right
>
>>> it and want the IOMMU capability. This is not used in this way here given how
>>> VFIODevice migration capability checking takes place *after* the device
>>> attachment.
>> Id on't understand the above sentence
>>
> The whole paragraph is meant to emphasize that we don't know if VF dirty
> tracking is supported because VFIODevice migration state hasn't been probed
> *yet*. And so we can't pick VF dirty tracking vs IOMMU dirty tracking at this
> stage when using IOMMU_HWPT_ALLOC_DIRTY_TRACKING flag and hence we always use it
> if IOMMU hw supports it even if later on VFIOMigration decides to use VF dirty
> tracking always instead.

that sounds a clearer explanation to me

Eric
>
>> Eric
>>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>>> ---
>>>  include/hw/vfio/vfio-common.h |  1 +
>>>  hw/vfio/iommufd.c             | 12 ++++++++++++
>>>  2 files changed, 13 insertions(+)
>>>
>>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
>>> index 2dd468ce3c02..760f31d84ac8 100644
>>> --- a/include/hw/vfio/vfio-common.h
>>> +++ b/include/hw/vfio/vfio-common.h
>>> @@ -97,6 +97,7 @@ typedef struct IOMMUFDBackend IOMMUFDBackend;
>>>  
>>>  typedef struct VFIOIOASHwpt {
>>>      uint32_t hwpt_id;
>>> +    uint32_t hwpt_flags;
>>>      QLIST_HEAD(, VFIODevice) device_list;
>>>      QLIST_ENTRY(VFIOIOASHwpt) next;
>>>  } VFIOIOASHwpt;
>>> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>>> index d34dc88231ec..edc8f97d8f3d 100644
>>> --- a/hw/vfio/iommufd.c
>>> +++ b/hw/vfio/iommufd.c
>>> @@ -246,6 +246,15 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
>>>          }
>>>      }
>>>  
>>> +    /*
>>> +     * This is quite early and VFIODevice isn't yet fully initialized,
>> so what's the problem exactly with the above?
> I should really say 'VFIO Migration state' here (see previous comment)
>
>>> +     * thus rely on IOMMU hardware capabilities as to whether IOMMU dirty
>>> +     * tracking is going to be needed.
>>> +     */
>>> +    if (vbasedev->hiod->caps.hw_caps & IOMMU_HW_CAP_DIRTY_TRACKING) {
>>> +        flags = IOMMU_HWPT_ALLOC_DIRTY_TRACKING;
>>> +    }
>>> +
>>>      if (!iommufd_backend_alloc_hwpt(iommufd, vbasedev->devid,
>>>                                      container->ioas_id, flags,
>>>                                      IOMMU_HWPT_DATA_NONE, 0, NULL,
>>> @@ -255,6 +264,7 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
>>>  
>>>      hwpt = g_malloc0(sizeof(*hwpt));
>>>      hwpt->hwpt_id = hwpt_id;
>>> +    hwpt->hwpt_flags = flags;
>>>      QLIST_INIT(&hwpt->device_list);
>>>  
>>>      ret = iommufd_cdev_attach_ioas_hwpt(vbasedev, hwpt->hwpt_id, errp);
>>> @@ -267,6 +277,8 @@ 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);
>>> +    container->bcontainer.dirty_pages_supported |=
>>> +                              (flags & IOMMU_HWPT_ALLOC_DIRTY_TRACKING);
>>>      return true;
>>>  }
>>>
Re: [PATCH v4 08/12] vfio/iommufd: Probe and request hwpt dirty tracking capability
Posted by Cédric Le Goater 4 months, 1 week ago 
On 7/12/24 13:47, Joao Martins wrote:
> Probe hardware dirty tracking support by querying device hw capabilities via
> IOMMUFD_GET_HW_INFO.
> 
> In preparation to using the dirty tracking UAPI, request dirty tracking in the
> HWPT flags when the IOMMU supports dirty tracking.
> 
> The auto domain logic allows different IOMMU domains to be created when DMA
> dirty tracking is not desired (and VF can provide it) while others doesn't have
> it and want the IOMMU capability. This is not used in this way here given how
> VFIODevice migration capability checking takes place *after* the device
> attachment.
> 
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> ---
>   include/hw/vfio/vfio-common.h |  1 +
>   hw/vfio/iommufd.c             | 12 ++++++++++++
>   2 files changed, 13 insertions(+)
> 
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index 2dd468ce3c02..760f31d84ac8 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -97,6 +97,7 @@ typedef struct IOMMUFDBackend IOMMUFDBackend;
>   
>   typedef struct VFIOIOASHwpt {
>       uint32_t hwpt_id;
> +    uint32_t hwpt_flags;
>       QLIST_HEAD(, VFIODevice) device_list;
>       QLIST_ENTRY(VFIOIOASHwpt) next;
>   } VFIOIOASHwpt;
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index d34dc88231ec..edc8f97d8f3d 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -246,6 +246,15 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
>           }
>       }
>   
> +    /*
> +     * This is quite early and VFIODevice isn't yet fully initialized,
> +     * thus rely on IOMMU hardware capabilities as to whether IOMMU dirty
> +     * tracking is going to be needed.
> +     */
> +    if (vbasedev->hiod->caps.hw_caps & IOMMU_HW_CAP_DIRTY_TRACKING) {
> +        flags = IOMMU_HWPT_ALLOC_DIRTY_TRACKING;
> +    }
> +
>       if (!iommufd_backend_alloc_hwpt(iommufd, vbasedev->devid,
>                                       container->ioas_id, flags,
>                                       IOMMU_HWPT_DATA_NONE, 0, NULL,
> @@ -255,6 +264,7 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
>   
>       hwpt = g_malloc0(sizeof(*hwpt));
>       hwpt->hwpt_id = hwpt_id;
> +    hwpt->hwpt_flags = flags;
>       QLIST_INIT(&hwpt->device_list);
>   
>       ret = iommufd_cdev_attach_ioas_hwpt(vbasedev, hwpt->hwpt_id, errp);
> @@ -267,6 +277,8 @@ 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);
> +    container->bcontainer.dirty_pages_supported |=
> +                              (flags & IOMMU_HWPT_ALLOC_DIRTY_TRACKING);
>       return true;
>   }
>   


Could you please introduce in this patch helper :

   static bool iommufd_hwpt_dirty_tracking(VFIOIOASHwpt *hwpt)
   {
       return hwpt->hwpt_flags & IOMMU_HWPT_ALLOC_DIRTY_TRACKING;
   }



Thanks,

C.

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.