diff mbox series

[v3,03/37] vfio/container: Move spapr specific init/deinit into spapr.c

Message ID 20231026103104.1686921-4-zhenzhong.duan@intel.com (mailing list archive)
State New, archived
Headers show
Series vfio: Adopt iommufd | expand

Commit Message

Duan, Zhenzhong Oct. 26, 2023, 10:30 a.m. UTC 
Move spapr specific init/deinit code into spapr.c and wrap
them with vfio_spapr_container_init/deinit, this way footprint
of spapr is further reduced, vfio_prereg_listener could also
be made static.

vfio_listener_release is unnecessary when prereg_listener is
moved out, so have it removed.

No functional changes intended.

Suggested-by: Cédric Le Goater <clg@redhat.com>
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 include/hw/vfio/vfio-common.h |  4 +-
 hw/vfio/container.c           | 82 +++++------------------------------
 hw/vfio/spapr.c               | 80 +++++++++++++++++++++++++++++++++-
 3 files changed, 94 insertions(+), 72 deletions(-)

Comments

Cédric Le Goater Oct. 27, 2023, 9:27 a.m. UTC | #1 
On 10/26/23 12:30, Zhenzhong Duan wrote:
> Move spapr specific init/deinit code into spapr.c and wrap
> them with vfio_spapr_container_init/deinit, this way footprint
> of spapr is further reduced, vfio_prereg_listener could also
> be made static.
> 
> vfio_listener_release is unnecessary when prereg_listener is
> moved out, so have it removed.
> 
> No functional changes intended.
> 
> Suggested-by: Cédric Le Goater <clg@redhat.com>
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   include/hw/vfio/vfio-common.h |  4 +-
>   hw/vfio/container.c           | 82 +++++------------------------------
>   hw/vfio/spapr.c               | 80 +++++++++++++++++++++++++++++++++-
>   3 files changed, 94 insertions(+), 72 deletions(-)
> 
> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
> index 0c3d390e8b..8d1d4f0a89 100644
> --- a/include/hw/vfio/vfio-common.h
> +++ b/include/hw/vfio/vfio-common.h
> @@ -225,11 +225,14 @@ int vfio_set_dirty_page_tracking(VFIOContainer *container, bool start);
>   int vfio_query_dirty_bitmap(VFIOContainer *container, VFIOBitmap *vbmap,
>                               hwaddr iova, hwaddr size);
>   
> +/* SPAPR specific */
>   int vfio_container_add_section_window(VFIOContainer *container,
>                                         MemoryRegionSection *section,
>                                         Error **errp);
>   void vfio_container_del_section_window(VFIOContainer *container,
>                                          MemoryRegionSection *section);
> +bool vfio_spapr_container_init(VFIOContainer *container, Error **errp);
> +void vfio_spapr_container_deinit(VFIOContainer *container);
>   
>   void vfio_disable_irqindex(VFIODevice *vbasedev, int index);
>   void vfio_unmask_single_irqindex(VFIODevice *vbasedev, int index);
> @@ -289,7 +292,6 @@ vfio_get_device_info_cap(struct vfio_device_info *info, uint16_t id);
>   struct vfio_info_cap_header *
>   vfio_get_cap(void *ptr, uint32_t cap_offset, uint16_t id);
>   #endif
> -extern const MemoryListener vfio_prereg_listener;
>   
>   int vfio_spapr_create_window(VFIOContainer *container,
>                                MemoryRegionSection *section,
> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
> index 7a3f005d1b..204b244b11 100644
> --- a/hw/vfio/container.c
> +++ b/hw/vfio/container.c
> @@ -264,14 +264,6 @@ int vfio_query_dirty_bitmap(VFIOContainer *container, VFIOBitmap *vbmap,
>       return ret;
>   }
>   
> -static void vfio_listener_release(VFIOContainer *container)
> -{
> -    memory_listener_unregister(&container->listener);
> -    if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU) {
> -        memory_listener_unregister(&container->prereg_listener);
> -    }
> -}
> -
>   static struct vfio_info_cap_header *
>   vfio_get_iommu_type1_info_cap(struct vfio_iommu_type1_info *info, uint16_t id)
>   {
> @@ -612,69 +604,11 @@ static int vfio_connect_container(VFIOGroup *group, AddressSpace *as,
>       case VFIO_SPAPR_TCE_v2_IOMMU:
>       case VFIO_SPAPR_TCE_IOMMU:
>       {
> -        struct vfio_iommu_spapr_tce_info info;
> -        bool v2 = container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU;
> -
> -        /*
> -         * The host kernel code implementing VFIO_IOMMU_DISABLE is called
> -         * when container fd is closed so we do not call it explicitly
> -         * in this file.
> -         */
> -        if (!v2) {
> -            ret = ioctl(fd, VFIO_IOMMU_ENABLE);
> -            if (ret) {
> -                error_setg_errno(errp, errno, "failed to enable container");
> -                ret = -errno;
> -                goto enable_discards_exit;
> -            }
> -        } else {
> -            container->prereg_listener = vfio_prereg_listener;
> -
> -            memory_listener_register(&container->prereg_listener,
> -                                     &address_space_memory);
> -            if (container->error) {
> -                memory_listener_unregister(&container->prereg_listener);
> -                ret = -1;
> -                error_propagate_prepend(errp, container->error,
> -                    "RAM memory listener initialization failed: ");
> -                goto enable_discards_exit;
> -            }
> -        }
> -
> -        info.argsz = sizeof(info);
> -        ret = ioctl(fd, VFIO_IOMMU_SPAPR_TCE_GET_INFO, &info);
> +        ret = vfio_spapr_container_init(container, errp);
>           if (ret) {
> -            error_setg_errno(errp, errno,
> -                             "VFIO_IOMMU_SPAPR_TCE_GET_INFO failed");
> -            ret = -errno;
> -            if (v2) {
> -                memory_listener_unregister(&container->prereg_listener);
> -            }
>               goto enable_discards_exit;
>           }
> -
> -        if (v2) {
> -            container->pgsizes = info.ddw.pgsizes;
> -            /*
> -             * There is a default window in just created container.
> -             * To make region_add/del simpler, we better remove this
> -             * window now and let those iommu_listener callbacks
> -             * create/remove them when needed.
> -             */
> -            ret = vfio_spapr_remove_window(container, info.dma32_window_start);
> -            if (ret) {
> -                error_setg_errno(errp, -ret,
> -                                 "failed to remove existing window");
> -                goto enable_discards_exit;
> -            }
> -        } else {
> -            /* The default table uses 4K pages */
> -            container->pgsizes = 0x1000;
> -            vfio_host_win_add(container, info.dma32_window_start,
> -                              info.dma32_window_start +
> -                              info.dma32_window_size - 1,
> -                              0x1000);
> -        }
> +        break;
>       }
>       }
>   
> @@ -704,7 +638,11 @@ listener_release_exit:
>       QLIST_REMOVE(group, container_next);
>       QLIST_REMOVE(container, next);
>       vfio_kvm_device_del_group(group);
> -    vfio_listener_release(container);
> +    memory_listener_unregister(&container->listener);
> +    if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU ||
> +        container->iommu_type == VFIO_SPAPR_TCE_IOMMU) {
> +        vfio_spapr_container_deinit(container);
> +    }
>   
>   enable_discards_exit:
>       vfio_ram_block_discard_disable(container, false);
> @@ -734,7 +672,11 @@ static void vfio_disconnect_container(VFIOGroup *group)
>        * group.
>        */
>       if (QLIST_EMPTY(&container->group_list)) {
> -        vfio_listener_release(container);
> +        memory_listener_unregister(&container->listener);
> +        if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU ||
> +            container->iommu_type == VFIO_SPAPR_TCE_IOMMU) {
> +            vfio_spapr_container_deinit(container);
> +        }
>       }
>   
>       if (ioctl(group->fd, VFIO_GROUP_UNSET_CONTAINER, &container->fd)) {
> diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
> index 9a7517c042..fd0613dfde 100644
> --- a/hw/vfio/spapr.c
> +++ b/hw/vfio/spapr.c
> @@ -15,6 +15,7 @@
>   #include <linux/kvm.h>
>   #endif
>   #include "sysemu/kvm.h"
> +#include "exec/address-spaces.h"
>   
>   #include "hw/vfio/vfio-common.h"
>   #include "hw/hw.h"
> @@ -139,7 +140,7 @@ static void vfio_prereg_listener_region_del(MemoryListener *listener,
>       trace_vfio_prereg_unregister(reg.vaddr, reg.size, ret ? -errno : 0);
>   }
>   
> -const MemoryListener vfio_prereg_listener = {
> +static const MemoryListener vfio_prereg_listener = {
>       .name = "vfio-pre-reg",
>       .region_add = vfio_prereg_listener_region_add,
>       .region_del = vfio_prereg_listener_region_del,
> @@ -343,3 +344,80 @@ void vfio_container_del_section_window(VFIOContainer *container,
>                    __func__, section->offset_within_address_space);
>       }
>   }
> +
> +bool vfio_spapr_container_init(VFIOContainer *container, Error **errp)
> +{
> +
> +    struct vfio_iommu_spapr_tce_info info;
> +    bool v2 = container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU;
> +    int ret, fd = container->fd;
> +
> +    /*
> +     * The host kernel code implementing VFIO_IOMMU_DISABLE is called
> +     * when container fd is closed so we do not call it explicitly
> +     * in this file.
> +     */
> +    if (!v2) {
> +        ret = ioctl(fd, VFIO_IOMMU_ENABLE);
> +        if (ret) {
> +            error_setg_errno(errp, errno, "failed to enable container");
> +            return -errno;
> +        }
> +    } else {
> +        container->prereg_listener = vfio_prereg_listener;
> +
> +        memory_listener_register(&container->prereg_listener,
> +                                 &address_space_memory);
> +        if (container->error) {
> +            ret = -1;
> +            error_propagate_prepend(errp, container->error,
> +                    "RAM memory listener initialization failed: ");
> +            goto listener_unregister_exit;
> +        }
> +    }
> +
> +    info.argsz = sizeof(info);
> +    ret = ioctl(fd, VFIO_IOMMU_SPAPR_TCE_GET_INFO, &info);
> +    if (ret) {
> +        error_setg_errno(errp, errno,
> +                         "VFIO_IOMMU_SPAPR_TCE_GET_INFO failed");
> +        ret = -errno;
> +        goto listener_unregister_exit;
> +    }
> +
> +    if (v2) {
> +        container->pgsizes = info.ddw.pgsizes;
> +        /*
> +         * There is a default window in just created container.
> +         * To make region_add/del simpler, we better remove this
> +         * window now and let those iommu_listener callbacks
> +         * create/remove them when needed.
> +         */
> +        ret = vfio_spapr_remove_window(container, info.dma32_window_start);
> +        if (ret) {
> +            error_setg_errno(errp, -ret,
> +                             "failed to remove existing window");
> +            goto listener_unregister_exit;
> +        }
> +    } else {
> +        /* The default table uses 4K pages */
> +        container->pgsizes = 0x1000;
> +        vfio_host_win_add(container, info.dma32_window_start,
> +                          info.dma32_window_start +
> +                          info.dma32_window_size - 1,
> +                          0x1000);
> +    }
> +
> +listener_unregister_exit:
> +    if (v2) {
> +        memory_listener_unregister(&container->prereg_listener);
> +    }
> +    return ret;
> +}
> +
> +void vfio_spapr_container_deinit(VFIOContainer *container)
> +{
> +    if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU) {
> +        memory_listener_unregister(&container->prereg_listener);
> +    }
> +}
diff mbox series

Patch

diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 0c3d390e8b..8d1d4f0a89 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -225,11 +225,14 @@  int vfio_set_dirty_page_tracking(VFIOContainer *container, bool start);
 int vfio_query_dirty_bitmap(VFIOContainer *container, VFIOBitmap *vbmap,
                             hwaddr iova, hwaddr size);
 
+/* SPAPR specific */
 int vfio_container_add_section_window(VFIOContainer *container,
                                       MemoryRegionSection *section,
                                       Error **errp);
 void vfio_container_del_section_window(VFIOContainer *container,
                                        MemoryRegionSection *section);
+bool vfio_spapr_container_init(VFIOContainer *container, Error **errp);
+void vfio_spapr_container_deinit(VFIOContainer *container);
 
 void vfio_disable_irqindex(VFIODevice *vbasedev, int index);
 void vfio_unmask_single_irqindex(VFIODevice *vbasedev, int index);
@@ -289,7 +292,6 @@  vfio_get_device_info_cap(struct vfio_device_info *info, uint16_t id);
 struct vfio_info_cap_header *
 vfio_get_cap(void *ptr, uint32_t cap_offset, uint16_t id);
 #endif
-extern const MemoryListener vfio_prereg_listener;
 
 int vfio_spapr_create_window(VFIOContainer *container,
                              MemoryRegionSection *section,
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 7a3f005d1b..204b244b11 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -264,14 +264,6 @@  int vfio_query_dirty_bitmap(VFIOContainer *container, VFIOBitmap *vbmap,
     return ret;
 }
 
-static void vfio_listener_release(VFIOContainer *container)
-{
-    memory_listener_unregister(&container->listener);
-    if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU) {
-        memory_listener_unregister(&container->prereg_listener);
-    }
-}
-
 static struct vfio_info_cap_header *
 vfio_get_iommu_type1_info_cap(struct vfio_iommu_type1_info *info, uint16_t id)
 {
@@ -612,69 +604,11 @@  static int vfio_connect_container(VFIOGroup *group, AddressSpace *as,
     case VFIO_SPAPR_TCE_v2_IOMMU:
     case VFIO_SPAPR_TCE_IOMMU:
     {
-        struct vfio_iommu_spapr_tce_info info;
-        bool v2 = container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU;
-
-        /*
-         * The host kernel code implementing VFIO_IOMMU_DISABLE is called
-         * when container fd is closed so we do not call it explicitly
-         * in this file.
-         */
-        if (!v2) {
-            ret = ioctl(fd, VFIO_IOMMU_ENABLE);
-            if (ret) {
-                error_setg_errno(errp, errno, "failed to enable container");
-                ret = -errno;
-                goto enable_discards_exit;
-            }
-        } else {
-            container->prereg_listener = vfio_prereg_listener;
-
-            memory_listener_register(&container->prereg_listener,
-                                     &address_space_memory);
-            if (container->error) {
-                memory_listener_unregister(&container->prereg_listener);
-                ret = -1;
-                error_propagate_prepend(errp, container->error,
-                    "RAM memory listener initialization failed: ");
-                goto enable_discards_exit;
-            }
-        }
-
-        info.argsz = sizeof(info);
-        ret = ioctl(fd, VFIO_IOMMU_SPAPR_TCE_GET_INFO, &info);
+        ret = vfio_spapr_container_init(container, errp);
         if (ret) {
-            error_setg_errno(errp, errno,
-                             "VFIO_IOMMU_SPAPR_TCE_GET_INFO failed");
-            ret = -errno;
-            if (v2) {
-                memory_listener_unregister(&container->prereg_listener);
-            }
             goto enable_discards_exit;
         }
-
-        if (v2) {
-            container->pgsizes = info.ddw.pgsizes;
-            /*
-             * There is a default window in just created container.
-             * To make region_add/del simpler, we better remove this
-             * window now and let those iommu_listener callbacks
-             * create/remove them when needed.
-             */
-            ret = vfio_spapr_remove_window(container, info.dma32_window_start);
-            if (ret) {
-                error_setg_errno(errp, -ret,
-                                 "failed to remove existing window");
-                goto enable_discards_exit;
-            }
-        } else {
-            /* The default table uses 4K pages */
-            container->pgsizes = 0x1000;
-            vfio_host_win_add(container, info.dma32_window_start,
-                              info.dma32_window_start +
-                              info.dma32_window_size - 1,
-                              0x1000);
-        }
+        break;
     }
     }
 
@@ -704,7 +638,11 @@  listener_release_exit:
     QLIST_REMOVE(group, container_next);
     QLIST_REMOVE(container, next);
     vfio_kvm_device_del_group(group);
-    vfio_listener_release(container);
+    memory_listener_unregister(&container->listener);
+    if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU ||
+        container->iommu_type == VFIO_SPAPR_TCE_IOMMU) {
+        vfio_spapr_container_deinit(container);
+    }
 
 enable_discards_exit:
     vfio_ram_block_discard_disable(container, false);
@@ -734,7 +672,11 @@  static void vfio_disconnect_container(VFIOGroup *group)
      * group.
      */
     if (QLIST_EMPTY(&container->group_list)) {
-        vfio_listener_release(container);
+        memory_listener_unregister(&container->listener);
+        if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU ||
+            container->iommu_type == VFIO_SPAPR_TCE_IOMMU) {
+            vfio_spapr_container_deinit(container);
+        }
     }
 
     if (ioctl(group->fd, VFIO_GROUP_UNSET_CONTAINER, &container->fd)) {
diff --git a/hw/vfio/spapr.c b/hw/vfio/spapr.c
index 9a7517c042..fd0613dfde 100644
--- a/hw/vfio/spapr.c
+++ b/hw/vfio/spapr.c
@@ -15,6 +15,7 @@ 
 #include <linux/kvm.h>
 #endif
 #include "sysemu/kvm.h"
+#include "exec/address-spaces.h"
 
 #include "hw/vfio/vfio-common.h"
 #include "hw/hw.h"
@@ -139,7 +140,7 @@  static void vfio_prereg_listener_region_del(MemoryListener *listener,
     trace_vfio_prereg_unregister(reg.vaddr, reg.size, ret ? -errno : 0);
 }
 
-const MemoryListener vfio_prereg_listener = {
+static const MemoryListener vfio_prereg_listener = {
     .name = "vfio-pre-reg",
     .region_add = vfio_prereg_listener_region_add,
     .region_del = vfio_prereg_listener_region_del,
@@ -343,3 +344,80 @@  void vfio_container_del_section_window(VFIOContainer *container,
                  __func__, section->offset_within_address_space);
     }
 }
+
+bool vfio_spapr_container_init(VFIOContainer *container, Error **errp)
+{
+
+    struct vfio_iommu_spapr_tce_info info;
+    bool v2 = container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU;
+    int ret, fd = container->fd;
+
+    /*
+     * The host kernel code implementing VFIO_IOMMU_DISABLE is called
+     * when container fd is closed so we do not call it explicitly
+     * in this file.
+     */
+    if (!v2) {
+        ret = ioctl(fd, VFIO_IOMMU_ENABLE);
+        if (ret) {
+            error_setg_errno(errp, errno, "failed to enable container");
+            return -errno;
+        }
+    } else {
+        container->prereg_listener = vfio_prereg_listener;
+
+        memory_listener_register(&container->prereg_listener,
+                                 &address_space_memory);
+        if (container->error) {
+            ret = -1;
+            error_propagate_prepend(errp, container->error,
+                    "RAM memory listener initialization failed: ");
+            goto listener_unregister_exit;
+        }
+    }
+
+    info.argsz = sizeof(info);
+    ret = ioctl(fd, VFIO_IOMMU_SPAPR_TCE_GET_INFO, &info);
+    if (ret) {
+        error_setg_errno(errp, errno,
+                         "VFIO_IOMMU_SPAPR_TCE_GET_INFO failed");
+        ret = -errno;
+        goto listener_unregister_exit;
+    }
+
+    if (v2) {
+        container->pgsizes = info.ddw.pgsizes;
+        /*
+         * There is a default window in just created container.
+         * To make region_add/del simpler, we better remove this
+         * window now and let those iommu_listener callbacks
+         * create/remove them when needed.
+         */
+        ret = vfio_spapr_remove_window(container, info.dma32_window_start);
+        if (ret) {
+            error_setg_errno(errp, -ret,
+                             "failed to remove existing window");
+            goto listener_unregister_exit;
+        }
+    } else {
+        /* The default table uses 4K pages */
+        container->pgsizes = 0x1000;
+        vfio_host_win_add(container, info.dma32_window_start,
+                          info.dma32_window_start +
+                          info.dma32_window_size - 1,
+                          0x1000);
+    }
+
+listener_unregister_exit:
+    if (v2) {
+        memory_listener_unregister(&container->prereg_listener);
+    }
+    return ret;
+}
+
+void vfio_spapr_container_deinit(VFIOContainer *container)
+{
+    if (container->iommu_type == VFIO_SPAPR_TCE_v2_IOMMU) {
+        memory_listener_unregister(&container->prereg_listener);
+    }
+}