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);
> + }
> +}