[PATCH v4 02/12] vfio/iommufd: Don't initialize nor set a HOST_IOMMU_DEVICE with mdev

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 02/12] vfio/iommufd: Don't initialize nor set a HOST_IOMMU_DEVICE with mdev
Posted by Joao Martins 4 months, 2 weeks ago
mdevs aren't "physical" devices and when asking for backing IOMMU info, it
fails the entire provisioning of the guest. Fix that by skipping
HostIOMMUDevice initialization in the presence of mdevs, and skip setting
an iommu device when it is known to be an mdev.

Cc: Zhenzhong Duan <zhenzhong.duan@intel.com>
Fixes: 930589520128 ("vfio/iommufd: Implement HostIOMMUDeviceClass::realize() handler")
Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
---
 hw/vfio/common.c |  4 ++++
 hw/vfio/pci.c    | 10 +++++++---
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/hw/vfio/common.c b/hw/vfio/common.c
index 7cdb969fd396..b0beed44116e 100644
--- a/hw/vfio/common.c
+++ b/hw/vfio/common.c
@@ -1556,6 +1556,10 @@ bool vfio_attach_device(char *name, VFIODevice *vbasedev,
         return false;
     }
 
+    if (vbasedev->mdev) {
+        return true;
+    }
+
     hiod = HOST_IOMMU_DEVICE(object_new(ops->hiod_typename));
     if (!HOST_IOMMU_DEVICE_GET_CLASS(hiod)->realize(hiod, vbasedev, errp)) {
         object_unref(hiod);
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 585f23a18406..3fc72e898a25 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -3116,7 +3116,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
 
     vfio_bars_register(vdev);
 
-    if (!pci_device_set_iommu_device(pdev, vbasedev->hiod, errp)) {
+    if (!is_mdev && !pci_device_set_iommu_device(pdev, vbasedev->hiod, errp)) {
         error_prepend(errp, "Failed to set iommu_device: ");
         goto out_teardown;
     }
@@ -3239,7 +3239,9 @@ out_deregister:
         timer_free(vdev->intx.mmap_timer);
     }
 out_unset_idev:
-    pci_device_unset_iommu_device(pdev);
+    if (!is_mdev) {
+        pci_device_unset_iommu_device(pdev);
+    }
 out_teardown:
     vfio_teardown_msi(vdev);
     vfio_bars_exit(vdev);
@@ -3284,7 +3286,9 @@ static void vfio_exitfn(PCIDevice *pdev)
     vfio_pci_disable_rp_atomics(vdev);
     vfio_bars_exit(vdev);
     vfio_migration_exit(vbasedev);
-    pci_device_unset_iommu_device(pdev);
+    if (!vbasedev->mdev) {
+        pci_device_unset_iommu_device(pdev);
+    }
 }
 
 static void vfio_pci_reset(DeviceState *dev)
-- 
2.17.2
RE: [PATCH v4 02/12] vfio/iommufd: Don't initialize nor set a HOST_IOMMU_DEVICE with mdev
Posted by Duan, Zhenzhong 4 months, 1 week ago
Hello Joao,

>-----Original Message-----
>From: Joao Martins <joao.m.martins@oracle.com>
>Subject: [PATCH v4 02/12] vfio/iommufd: Don't initialize nor set a
>HOST_IOMMU_DEVICE with mdev
>
>mdevs aren't "physical" devices and when asking for backing IOMMU info, it
>fails the entire provisioning of the guest. Fix that by skipping
>HostIOMMUDevice initialization in the presence of mdevs, and skip setting
>an iommu device when it is known to be an mdev.
>
>Cc: Zhenzhong Duan <zhenzhong.duan@intel.com>
>Fixes: 930589520128 ("vfio/iommufd: Implement
>HostIOMMUDeviceClass::realize() handler")
>Signed-off-by: Joao Martins <joao.m.martins@oracle.com>

Thanks for fixing.

Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>

 BRs.
Zhenzhong

>---
> hw/vfio/common.c |  4 ++++
> hw/vfio/pci.c    | 10 +++++++---
> 2 files changed, 11 insertions(+), 3 deletions(-)
>
>diff --git a/hw/vfio/common.c b/hw/vfio/common.c
>index 7cdb969fd396..b0beed44116e 100644
>--- a/hw/vfio/common.c
>+++ b/hw/vfio/common.c
>@@ -1556,6 +1556,10 @@ bool vfio_attach_device(char *name,
>VFIODevice *vbasedev,
>         return false;
>     }
>
>+    if (vbasedev->mdev) {
>+        return true;
>+    }
>+
>     hiod = HOST_IOMMU_DEVICE(object_new(ops->hiod_typename));
>     if (!HOST_IOMMU_DEVICE_GET_CLASS(hiod)->realize(hiod, vbasedev,
>errp)) {
>         object_unref(hiod);
>diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>index 585f23a18406..3fc72e898a25 100644
>--- a/hw/vfio/pci.c
>+++ b/hw/vfio/pci.c
>@@ -3116,7 +3116,7 @@ static void vfio_realize(PCIDevice *pdev, Error
>**errp)
>
>     vfio_bars_register(vdev);
>
>-    if (!pci_device_set_iommu_device(pdev, vbasedev->hiod, errp)) {
>+    if (!is_mdev && !pci_device_set_iommu_device(pdev, vbasedev->hiod,
>errp)) {
>         error_prepend(errp, "Failed to set iommu_device: ");
>         goto out_teardown;
>     }
>@@ -3239,7 +3239,9 @@ out_deregister:
>         timer_free(vdev->intx.mmap_timer);
>     }
> out_unset_idev:
>-    pci_device_unset_iommu_device(pdev);
>+    if (!is_mdev) {
>+        pci_device_unset_iommu_device(pdev);
>+    }
> out_teardown:
>     vfio_teardown_msi(vdev);
>     vfio_bars_exit(vdev);
>@@ -3284,7 +3286,9 @@ static void vfio_exitfn(PCIDevice *pdev)
>     vfio_pci_disable_rp_atomics(vdev);
>     vfio_bars_exit(vdev);
>     vfio_migration_exit(vbasedev);
>-    pci_device_unset_iommu_device(pdev);
>+    if (!vbasedev->mdev) {
>+        pci_device_unset_iommu_device(pdev);
>+    }
> }
>
> static void vfio_pci_reset(DeviceState *dev)
>--
>2.17.2
Re: [PATCH v4 02/12] vfio/iommufd: Don't initialize nor set a HOST_IOMMU_DEVICE with mdev
Posted by Eric Auger 4 months, 1 week ago

On 7/12/24 13:46, Joao Martins wrote:
> mdevs aren't "physical" devices and when asking for backing IOMMU info, it
> fails the entire provisioning of the guest. Fix that by skipping
> HostIOMMUDevice initialization in the presence of mdevs, and skip setting
> an iommu device when it is known to be an mdev.
>
> Cc: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Fixes: 930589520128 ("vfio/iommufd: Implement HostIOMMUDeviceClass::realize() handler")
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
With or without Cédric's suggestion
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Eric
> ---
>  hw/vfio/common.c |  4 ++++
>  hw/vfio/pci.c    | 10 +++++++---
>  2 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 7cdb969fd396..b0beed44116e 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -1556,6 +1556,10 @@ bool vfio_attach_device(char *name, VFIODevice *vbasedev,
>          return false;
>      }
>  
> +    if (vbasedev->mdev) {
> +        return true;
> +    }
> +
>      hiod = HOST_IOMMU_DEVICE(object_new(ops->hiod_typename));
>      if (!HOST_IOMMU_DEVICE_GET_CLASS(hiod)->realize(hiod, vbasedev, errp)) {
>          object_unref(hiod);
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 585f23a18406..3fc72e898a25 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -3116,7 +3116,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
>  
>      vfio_bars_register(vdev);
>  
> -    if (!pci_device_set_iommu_device(pdev, vbasedev->hiod, errp)) {
> +    if (!is_mdev && !pci_device_set_iommu_device(pdev, vbasedev->hiod, errp)) {
>          error_prepend(errp, "Failed to set iommu_device: ");
>          goto out_teardown;
>      }
> @@ -3239,7 +3239,9 @@ out_deregister:
>          timer_free(vdev->intx.mmap_timer);
>      }
>  out_unset_idev:
> -    pci_device_unset_iommu_device(pdev);
> +    if (!is_mdev) {
> +        pci_device_unset_iommu_device(pdev);
> +    }
>  out_teardown:
>      vfio_teardown_msi(vdev);
>      vfio_bars_exit(vdev);
> @@ -3284,7 +3286,9 @@ static void vfio_exitfn(PCIDevice *pdev)
>      vfio_pci_disable_rp_atomics(vdev);
>      vfio_bars_exit(vdev);
>      vfio_migration_exit(vbasedev);
> -    pci_device_unset_iommu_device(pdev);
> +    if (!vbasedev->mdev) {
> +        pci_device_unset_iommu_device(pdev);
> +    }
>  }
>  
>  static void vfio_pci_reset(DeviceState *dev)


Re: [PATCH v4 02/12] vfio/iommufd: Don't initialize nor set a HOST_IOMMU_DEVICE with mdev
Posted by Cédric Le Goater 4 months, 1 week ago
On 7/12/24 13:46, Joao Martins wrote:
> mdevs aren't "physical" devices and when asking for backing IOMMU info, it
> fails the entire provisioning of the guest. Fix that by skipping
> HostIOMMUDevice initialization in the presence of mdevs, and skip setting
> an iommu device when it is known to be an mdev.
> 
> Cc: Zhenzhong Duan <zhenzhong.duan@intel.com>
> Fixes: 930589520128 ("vfio/iommufd: Implement HostIOMMUDeviceClass::realize() handler")
> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
> ---
>   hw/vfio/common.c |  4 ++++
>   hw/vfio/pci.c    | 10 +++++++---
>   2 files changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> index 7cdb969fd396..b0beed44116e 100644
> --- a/hw/vfio/common.c
> +++ b/hw/vfio/common.c
> @@ -1556,6 +1556,10 @@ bool vfio_attach_device(char *name, VFIODevice *vbasedev,
>           return false;
>       }
>   
> +    if (vbasedev->mdev) {
> +        return true;
> +    }
> +
>       hiod = HOST_IOMMU_DEVICE(object_new(ops->hiod_typename));
>       if (!HOST_IOMMU_DEVICE_GET_CLASS(hiod)->realize(hiod, vbasedev, errp)) {
>           object_unref(hiod);
> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
> index 585f23a18406..3fc72e898a25 100644
> --- a/hw/vfio/pci.c
> +++ b/hw/vfio/pci.c
> @@ -3116,7 +3116,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
>   
>       vfio_bars_register(vdev);
>   
> -    if (!pci_device_set_iommu_device(pdev, vbasedev->hiod, errp)) {
> +    if (!is_mdev && !pci_device_set_iommu_device(pdev, vbasedev->hiod, errp)) {

let's use vbasedev->mdev instead.


Thanks,

C.


>           error_prepend(errp, "Failed to set iommu_device: ");
>           goto out_teardown;
>       }
> @@ -3239,7 +3239,9 @@ out_deregister:
>           timer_free(vdev->intx.mmap_timer);
>       }
>   out_unset_idev:
> -    pci_device_unset_iommu_device(pdev);
> +    if (!is_mdev) {
> +        pci_device_unset_iommu_device(pdev);
> +    }
>   out_teardown:
>       vfio_teardown_msi(vdev);
>       vfio_bars_exit(vdev);
> @@ -3284,7 +3286,9 @@ static void vfio_exitfn(PCIDevice *pdev)
>       vfio_pci_disable_rp_atomics(vdev);
>       vfio_bars_exit(vdev);
>       vfio_migration_exit(vbasedev);
> -    pci_device_unset_iommu_device(pdev);
> +    if (!vbasedev->mdev) {
> +        pci_device_unset_iommu_device(pdev);
> +    }
>   }
>   
>   static void vfio_pci_reset(DeviceState *dev)