diff mbox series

[RFC,v3,18/18] vfio/as: Allow the selection of a given iommu backend

Message ID 20230131205305.2726330-19-eric.auger@redhat.com
State New
Headers show
Series vfio: Adopt iommufd | expand

Commit Message

Eric Auger Jan. 31, 2023, 8:53 p.m. UTC 
Now we support two types of iommu backends, let's add the capability
to select one of them. This depends on whether an iommufd object has
been linked with the vfio-pci device:

if the user wants to use the legacy backend, it shall not
link the vfio-pci device with any iommufd object:

-device vfio-pci,host=0000:02:00.0

This is called the legacy mode/backend.

If the user wants to use the iommufd backend (/dev/iommu) it
shall pass an iommufd object id in the vfio-pci device options:

 -object iommufd,id=iommufd0
 -device vfio-pci,host=0000:02:00.0,iommufd=iommufd0

Note the /dev/iommu device may have been pre-opened by a
management tool such as libvirt. This mode is no more considered
for the legacy backend. So let's remove the "TODO" comment.

Signed-off-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Yi Liu <yi.l.liu@intel.com>
Suggested-by: Alex Williamson <alex.williamson@redhat.com>
---
 hw/vfio/pci.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

Comments

Jason Gunthorpe Feb. 3, 2023, 12:51 p.m. UTC | #1 
On Tue, Jan 31, 2023 at 09:53:05PM +0100, Eric Auger wrote:
> Now we support two types of iommu backends, let's add the capability
> to select one of them. This depends on whether an iommufd object has
> been linked with the vfio-pci device:
> 
> if the user wants to use the legacy backend, it shall not
> link the vfio-pci device with any iommufd object:
> 
> -device vfio-pci,host=0000:02:00.0
> 
> This is called the legacy mode/backend.
> 
> If the user wants to use the iommufd backend (/dev/iommu) it
> shall pass an iommufd object id in the vfio-pci device options:
> 
>  -object iommufd,id=iommufd0
>  -device vfio-pci,host=0000:02:00.0,iommufd=iommufd0
> 
> Note the /dev/iommu device may have been pre-opened by a
> management tool such as libvirt. This mode is no more considered
> for the legacy backend. So let's remove the "TODO" comment.

The vfio cdev should also be pre-openable like iommufd?

Jason
Eric Auger Feb. 3, 2023, 5:57 p.m. UTC | #2 
Hi Jason,

On 2/3/23 13:51, Jason Gunthorpe wrote:
> On Tue, Jan 31, 2023 at 09:53:05PM +0100, Eric Auger wrote:
>> Now we support two types of iommu backends, let's add the capability
>> to select one of them. This depends on whether an iommufd object has
>> been linked with the vfio-pci device:
>>
>> if the user wants to use the legacy backend, it shall not
>> link the vfio-pci device with any iommufd object:
>>
>> -device vfio-pci,host=0000:02:00.0
>>
>> This is called the legacy mode/backend.
>>
>> If the user wants to use the iommufd backend (/dev/iommu) it
>> shall pass an iommufd object id in the vfio-pci device options:
>>
>>  -object iommufd,id=iommufd0
>>  -device vfio-pci,host=0000:02:00.0,iommufd=iommufd0
>>
>> Note the /dev/iommu device may have been pre-opened by a
>> management tool such as libvirt. This mode is no more considered
>> for the legacy backend. So let's remove the "TODO" comment.
> The vfio cdev should also be pre-openable like iommufd?

where does the requirement come from?

Thanks

Eric
>
> Jason
>
Jason Gunthorpe Feb. 3, 2023, 6:01 p.m. UTC | #3 
On Fri, Feb 03, 2023 at 06:57:02PM +0100, Eric Auger wrote:
> Hi Jason,
> 
> On 2/3/23 13:51, Jason Gunthorpe wrote:
> > On Tue, Jan 31, 2023 at 09:53:05PM +0100, Eric Auger wrote:
> >> Now we support two types of iommu backends, let's add the capability
> >> to select one of them. This depends on whether an iommufd object has
> >> been linked with the vfio-pci device:
> >>
> >> if the user wants to use the legacy backend, it shall not
> >> link the vfio-pci device with any iommufd object:
> >>
> >> -device vfio-pci,host=0000:02:00.0
> >>
> >> This is called the legacy mode/backend.
> >>
> >> If the user wants to use the iommufd backend (/dev/iommu) it
> >> shall pass an iommufd object id in the vfio-pci device options:
> >>
> >>  -object iommufd,id=iommufd0
> >>  -device vfio-pci,host=0000:02:00.0,iommufd=iommufd0
> >>
> >> Note the /dev/iommu device may have been pre-opened by a
> >> management tool such as libvirt. This mode is no more considered
> >> for the legacy backend. So let's remove the "TODO" comment.
> > The vfio cdev should also be pre-openable like iommufd?
> 
> where does the requirement come from?

I would expect it helps sandbox security.

We couldn't do this with the original VFIO model, but now we can have
libvirt open the vfio with privilege and FD pass it to qemu.

This way qemu never needs to have privilege to open a VFIO or iommu
cdev node.

Jason
diff mbox series

Patch

diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 8689b3053c..ebf8f8ca83 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -42,6 +42,7 @@ 
 #include "qapi/error.h"
 #include "migration/blocker.h"
 #include "migration/qemu-file.h"
+#include "sysemu/iommufd.h"
 
 #define TYPE_VFIO_PCI_NOHOTPLUG "vfio-pci-nohotplug"
 
@@ -2852,6 +2853,13 @@  static void vfio_realize(PCIDevice *pdev, Error **errp)
     int i, ret;
     bool is_mdev;
 
+    if (vbasedev->iommufd) {
+        iommufd_backend_connect(vbasedev->iommufd, errp);
+        if (*errp) {
+            return;
+        }
+    }
+
     if (!vbasedev->sysfsdev) {
         if (!(~vdev->host.domain || ~vdev->host.bus ||
               ~vdev->host.slot || ~vdev->host.function)) {
@@ -3134,6 +3142,7 @@  error:
 static void vfio_instance_finalize(Object *obj)
 {
     VFIOPCIDevice *vdev = VFIO_PCI(obj);
+    VFIODevice *vbasedev = &vdev->vbasedev;
 
     vfio_display_finalize(vdev);
     vfio_bars_finalize(vdev);
@@ -3146,6 +3155,9 @@  static void vfio_instance_finalize(Object *obj)
      *
      * g_free(vdev->igd_opregion);
      */
+    if (vbasedev->iommufd) {
+        iommufd_backend_disconnect(vbasedev->iommufd);
+    }
     vfio_put_device(vdev);
 }
 
@@ -3281,11 +3293,8 @@  static Property vfio_pci_dev_properties[] = {
                                    qdev_prop_nv_gpudirect_clique, uint8_t),
     DEFINE_PROP_OFF_AUTO_PCIBAR("x-msix-relocation", VFIOPCIDevice, msix_relo,
                                 OFF_AUTOPCIBAR_OFF),
-    /*
-     * TODO - support passed fds... is this necessary?
-     * DEFINE_PROP_STRING("vfiofd", VFIOPCIDevice, vfiofd_name),
-     * DEFINE_PROP_STRING("vfiogroupfd, VFIOPCIDevice, vfiogroupfd_name),
-     */
+    DEFINE_PROP_LINK("iommufd", VFIOPCIDevice, vbasedev.iommufd,
+                     TYPE_IOMMUFD_BACKEND, IOMMUFDBackend *),
     DEFINE_PROP_END_OF_LIST(),
 };