[PATCH v1 03/13] vfio/iommufd: Create nesting parent hwpt with IOMMU_HWPT_ALLOC_PASID flag

Zhenzhong Duan posted 13 patches 1 month ago
Maintainers: Yi Liu <yi.l.liu@intel.com>, Eric Auger <eric.auger@redhat.com>, Zhenzhong Duan <zhenzhong.duan@intel.com>, Peter Maydell <peter.maydell@linaro.org>, "Michael S. Tsirkin" <mst@redhat.com>, Jason Wang <jasowang@redhat.com>, "Clément Mathieu--Drif" <clement.mathieu--drif@eviden.com>, Marcel Apfelbaum <marcel.apfelbaum@gmail.com>, Paolo Bonzini <pbonzini@redhat.com>, Richard Henderson <richard.henderson@linaro.org>, Eduardo Habkost <eduardo@habkost.net>, Alex Williamson <alex@shazbot.org>, "Cédric Le Goater" <clg@redhat.com>
There is a newer version of this series
[PATCH v1 03/13] vfio/iommufd: Create nesting parent hwpt with IOMMU_HWPT_ALLOC_PASID flag
Posted by Zhenzhong Duan 1 month ago
When both host device and vIOMMU have PASID enabled, then guest may
setup pasid attached translation.

We need to create the nesting parent hwpt with IOMMU_HWPT_ALLOC_PASID
flag because according to uAPI, any domain attached to the non-PASID
part of the device must also be flagged, otherwise attaching a PASID
will blocked.

Introduce a vfio_device_get_viommu_flags_pasid_supported() helper to
facilitate this implementation.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 include/hw/vfio/vfio-device.h |  1 +
 hw/vfio/device.c              | 11 +++++++++++
 hw/vfio/iommufd.c             |  8 +++++++-
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/include/hw/vfio/vfio-device.h b/include/hw/vfio/vfio-device.h
index 828a31c006..dd0355eb3d 100644
--- a/include/hw/vfio/vfio-device.h
+++ b/include/hw/vfio/vfio-device.h
@@ -268,6 +268,7 @@ void vfio_device_prepare(VFIODevice *vbasedev, VFIOContainer *bcontainer,
 void vfio_device_unprepare(VFIODevice *vbasedev);
 
 bool vfio_device_get_viommu_flags_want_nesting(VFIODevice *vbasedev);
+bool vfio_device_get_viommu_flags_pasid_supported(VFIODevice *vbasedev);
 bool vfio_device_get_host_iommu_quirk_bypass_ro(VFIODevice *vbasedev,
                                                 uint32_t type, void *caps,
                                                 uint32_t size);
diff --git a/hw/vfio/device.c b/hw/vfio/device.c
index 973fc35b59..b15ca6ef0a 100644
--- a/hw/vfio/device.c
+++ b/hw/vfio/device.c
@@ -533,6 +533,17 @@ bool vfio_device_get_viommu_flags_want_nesting(VFIODevice *vbasedev)
     return false;
 }
 
+bool vfio_device_get_viommu_flags_pasid_supported(VFIODevice *vbasedev)
+{
+    VFIOPCIDevice *vdev = vfio_pci_from_vfio_device(vbasedev);
+
+    if (vdev) {
+        return !!(pci_device_get_viommu_flags(PCI_DEVICE(vdev)) &
+                  VIOMMU_FLAG_PASID_SUPPORTED);
+    }
+    return false;
+}
+
 bool vfio_device_get_host_iommu_quirk_bypass_ro(VFIODevice *vbasedev,
                                                 uint32_t type, void *caps,
                                                 uint32_t size)
diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index 005f97fe25..c408f9151b 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -360,6 +360,7 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
     VendorCaps caps;
     VFIOIOASHwpt *hwpt;
     uint32_t hwpt_id;
+    uint8_t max_pasid_log2 = 0;
     int ret;
 
     /* Try to find a domain */
@@ -405,7 +406,7 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
      */
     if (!iommufd_backend_get_device_info(vbasedev->iommufd, vbasedev->devid,
                                          &type, &caps, sizeof(caps), &hw_caps,
-                                         NULL, errp)) {
+                                         &max_pasid_log2, errp)) {
         return false;
     }
 
@@ -427,6 +428,11 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
         }
     }
 
+    if (max_pasid_log2 &&
+        vfio_device_get_viommu_flags_pasid_supported(vbasedev)) {
+        flags |= IOMMU_HWPT_ALLOC_PASID;
+    }
+
     if (cpr_is_incoming()) {
         hwpt_id = vbasedev->cpr.hwpt_id;
         goto skip_alloc;
-- 
2.47.3
Re: [PATCH v1 03/13] vfio/iommufd: Create nesting parent hwpt with IOMMU_HWPT_ALLOC_PASID flag
Posted by Yi Liu 3 weeks, 3 days ago
On 3/6/26 11:43, Zhenzhong Duan wrote:
> When both host device and vIOMMU have PASID enabled, then guest may
> setup pasid attached translation.

Better be "When both both device and vIOMMU". host device enabled pasid
does not mean QEMU will for sure report pasid PCI ecap to guest. :)

> We need to create the nesting parent hwpt with IOMMU_HWPT_ALLOC_PASID
> flag because according to uAPI, any domain attached to the non-PASID
> part of the device must also be flagged, otherwise attaching a PASID
> will blocked.

This may make me thinking about why nested parent hwpt is special here.
I think you just call out vfio needs to be aware of potential pasid
usage and should attach the non-pasid part of pasid-capable device to
hwpt flagged with IOMMU_HWPT_ALLOC_PASID.

Code change LGTM.

Reviewed-by: Yi Liu <yi.l.liu@intel.com>

> Introduce a vfio_device_get_viommu_flags_pasid_supported() helper to
> facilitate this implementation.
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>   include/hw/vfio/vfio-device.h |  1 +
>   hw/vfio/device.c              | 11 +++++++++++
>   hw/vfio/iommufd.c             |  8 +++++++-
>   3 files changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/include/hw/vfio/vfio-device.h b/include/hw/vfio/vfio-device.h
> index 828a31c006..dd0355eb3d 100644
> --- a/include/hw/vfio/vfio-device.h
> +++ b/include/hw/vfio/vfio-device.h
> @@ -268,6 +268,7 @@ void vfio_device_prepare(VFIODevice *vbasedev, VFIOContainer *bcontainer,
>   void vfio_device_unprepare(VFIODevice *vbasedev);
>   
>   bool vfio_device_get_viommu_flags_want_nesting(VFIODevice *vbasedev);
> +bool vfio_device_get_viommu_flags_pasid_supported(VFIODevice *vbasedev);
>   bool vfio_device_get_host_iommu_quirk_bypass_ro(VFIODevice *vbasedev,
>                                                   uint32_t type, void *caps,
>                                                   uint32_t size);
> diff --git a/hw/vfio/device.c b/hw/vfio/device.c
> index 973fc35b59..b15ca6ef0a 100644
> --- a/hw/vfio/device.c
> +++ b/hw/vfio/device.c
> @@ -533,6 +533,17 @@ bool vfio_device_get_viommu_flags_want_nesting(VFIODevice *vbasedev)
>       return false;
>   }
>   
> +bool vfio_device_get_viommu_flags_pasid_supported(VFIODevice *vbasedev)
> +{
> +    VFIOPCIDevice *vdev = vfio_pci_from_vfio_device(vbasedev);
> +
> +    if (vdev) {
> +        return !!(pci_device_get_viommu_flags(PCI_DEVICE(vdev)) &
> +                  VIOMMU_FLAG_PASID_SUPPORTED);
> +    }
> +    return false;
> +}
> +
>   bool vfio_device_get_host_iommu_quirk_bypass_ro(VFIODevice *vbasedev,
>                                                   uint32_t type, void *caps,
>                                                   uint32_t size)
> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> index 005f97fe25..c408f9151b 100644
> --- a/hw/vfio/iommufd.c
> +++ b/hw/vfio/iommufd.c
> @@ -360,6 +360,7 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
>       VendorCaps caps;
>       VFIOIOASHwpt *hwpt;
>       uint32_t hwpt_id;
> +    uint8_t max_pasid_log2 = 0;
>       int ret;
>   
>       /* Try to find a domain */
> @@ -405,7 +406,7 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
>        */
>       if (!iommufd_backend_get_device_info(vbasedev->iommufd, vbasedev->devid,
>                                            &type, &caps, sizeof(caps), &hw_caps,
> -                                         NULL, errp)) {
> +                                         &max_pasid_log2, errp)) {
>           return false;
>       }
>   
> @@ -427,6 +428,11 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
>           }
>       }
>   
> +    if (max_pasid_log2 &&
> +        vfio_device_get_viommu_flags_pasid_supported(vbasedev)) {
> +        flags |= IOMMU_HWPT_ALLOC_PASID;
> +    }
> +
>       if (cpr_is_incoming()) {
>           hwpt_id = vbasedev->cpr.hwpt_id;
>           goto skip_alloc;
RE: [PATCH v1 03/13] vfio/iommufd: Create nesting parent hwpt with IOMMU_HWPT_ALLOC_PASID flag
Posted by Duan, Zhenzhong 3 weeks, 2 days ago

>-----Original Message-----
>From: Liu, Yi L <yi.l.liu@intel.com>
>Subject: Re: [PATCH v1 03/13] vfio/iommufd: Create nesting parent hwpt with
>IOMMU_HWPT_ALLOC_PASID flag
>
>On 3/6/26 11:43, Zhenzhong Duan wrote:
>> When both host device and vIOMMU have PASID enabled, then guest may
>> setup pasid attached translation.
>
>Better be "When both both device and vIOMMU". host device enabled pasid
>does not mean QEMU will for sure report pasid PCI ecap to guest. :)

Sure.

>
>> We need to create the nesting parent hwpt with IOMMU_HWPT_ALLOC_PASID
>> flag because according to uAPI, any domain attached to the non-PASID
>> part of the device must also be flagged, otherwise attaching a PASID
>> will blocked.
>
>This may make me thinking about why nested parent hwpt is special here.
>I think you just call out vfio needs to be aware of potential pasid
>usage and should attach the non-pasid part of pasid-capable device to
>hwpt flagged with IOMMU_HWPT_ALLOC_PASID.

Exactly.

Thanks
Zhenzhong

>
>Code change LGTM.
>
>Reviewed-by: Yi Liu <yi.l.liu@intel.com>
>
>> Introduce a vfio_device_get_viommu_flags_pasid_supported() helper to
>> facilitate this implementation.
>>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>   include/hw/vfio/vfio-device.h |  1 +
>>   hw/vfio/device.c              | 11 +++++++++++
>>   hw/vfio/iommufd.c             |  8 +++++++-
>>   3 files changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/hw/vfio/vfio-device.h b/include/hw/vfio/vfio-device.h
>> index 828a31c006..dd0355eb3d 100644
>> --- a/include/hw/vfio/vfio-device.h
>> +++ b/include/hw/vfio/vfio-device.h
>> @@ -268,6 +268,7 @@ void vfio_device_prepare(VFIODevice *vbasedev,
>VFIOContainer *bcontainer,
>>   void vfio_device_unprepare(VFIODevice *vbasedev);
>>
>>   bool vfio_device_get_viommu_flags_want_nesting(VFIODevice *vbasedev);
>> +bool vfio_device_get_viommu_flags_pasid_supported(VFIODevice *vbasedev);
>>   bool vfio_device_get_host_iommu_quirk_bypass_ro(VFIODevice *vbasedev,
>>                                                   uint32_t type, void *caps,
>>                                                   uint32_t size);
>> diff --git a/hw/vfio/device.c b/hw/vfio/device.c
>> index 973fc35b59..b15ca6ef0a 100644
>> --- a/hw/vfio/device.c
>> +++ b/hw/vfio/device.c
>> @@ -533,6 +533,17 @@ bool
>vfio_device_get_viommu_flags_want_nesting(VFIODevice *vbasedev)
>>       return false;
>>   }
>>
>> +bool vfio_device_get_viommu_flags_pasid_supported(VFIODevice *vbasedev)
>> +{
>> +    VFIOPCIDevice *vdev = vfio_pci_from_vfio_device(vbasedev);
>> +
>> +    if (vdev) {
>> +        return !!(pci_device_get_viommu_flags(PCI_DEVICE(vdev)) &
>> +                  VIOMMU_FLAG_PASID_SUPPORTED);
>> +    }
>> +    return false;
>> +}
>> +
>>   bool vfio_device_get_host_iommu_quirk_bypass_ro(VFIODevice *vbasedev,
>>                                                   uint32_t type, void *caps,
>>                                                   uint32_t size)
>> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
>> index 005f97fe25..c408f9151b 100644
>> --- a/hw/vfio/iommufd.c
>> +++ b/hw/vfio/iommufd.c
>> @@ -360,6 +360,7 @@ static bool
>iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
>>       VendorCaps caps;
>>       VFIOIOASHwpt *hwpt;
>>       uint32_t hwpt_id;
>> +    uint8_t max_pasid_log2 = 0;
>>       int ret;
>>
>>       /* Try to find a domain */
>> @@ -405,7 +406,7 @@ static bool
>iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
>>        */
>>       if (!iommufd_backend_get_device_info(vbasedev->iommufd, vbasedev-
>>devid,
>>                                            &type, &caps, sizeof(caps), &hw_caps,
>> -                                         NULL, errp)) {
>> +                                         &max_pasid_log2, errp)) {
>>           return false;
>>       }
>>
>> @@ -427,6 +428,11 @@ static bool
>iommufd_cdev_autodomains_get(VFIODevice *vbasedev,
>>           }
>>       }
>>
>> +    if (max_pasid_log2 &&
>> +        vfio_device_get_viommu_flags_pasid_supported(vbasedev)) {
>> +        flags |= IOMMU_HWPT_ALLOC_PASID;
>> +    }
>> +
>>       if (cpr_is_incoming()) {
>>           hwpt_id = vbasedev->cpr.hwpt_id;
>>           goto skip_alloc;