Re: [PATCH V4 11/14] vfio: register container for cpr

[Cédric Le Goater]

Hello Steve,

On 2/22/24 18:28, Steve Sistare wrote:

Define entry points to perform per-container cpr-specific initialization
and teardown.

Signed-off-by: Steve Sistare <steven.sist...@oracle.com>
---
  hw/vfio/container.c           | 11 ++++++++++-
  hw/vfio/cpr.c                 | 19 +++++++++++++++++++
  hw/vfio/iommufd.c             |  6 ++++++
  hw/vfio/meson.build           |  1 +
  include/hw/vfio/vfio-common.h |  3 +++
  5 files changed, 39 insertions(+), 1 deletion(-)
  create mode 100644 hw/vfio/cpr.c

diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index bd25b9f..096d77e 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -621,10 +621,15 @@ static int vfio_connect_container(VFIOGroup *group, 
AddressSpace *as,
          goto free_container_exit;
      }

+ ret = vfio_cpr_register_container(bcontainer, errp);

+    if (ret) {
+        goto free_container_exit;
+    }
+
      ret = vfio_ram_block_discard_disable(container, true);
      if (ret) {
          error_setg_errno(errp, -ret, "Cannot set discarding of RAM broken");
-        goto free_container_exit;
+        goto unregister_container_exit;
      }

assert(bcontainer->ops->setup);

@@ -667,6 +672,9 @@ listener_release_exit:
  enable_discards_exit:
      vfio_ram_block_discard_disable(container, false);

+unregister_container_exit:

+    vfio_cpr_unregister_container(bcontainer);
+
  free_container_exit:
      g_free(container);

@@ -710,6 +718,7 @@ static void vfio_disconnect_container(VFIOGroup *group)

          vfio_container_destroy(bcontainer);

trace_vfio_disconnect_container(container->fd);

+        vfio_cpr_unregister_container(bcontainer);
          close(container->fd);
          g_free(container);

diff --git a/hw/vfio/cpr.c b/hw/vfio/cpr.c

new file mode 100644
index 0000000..3bede54
--- /dev/null
+++ b/hw/vfio/cpr.c
@@ -0,0 +1,19 @@
+/*
+ * Copyright (c) 2021-2024 Oracle and/or its affiliates.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "hw/vfio/vfio-common.h"
+#include "qapi/error.h"
+
+int vfio_cpr_register_container(VFIOContainerBase *bcontainer, Error **errp)
+{
+    return 0;
+}
+
+void vfio_cpr_unregister_container(VFIOContainerBase *bcontainer)
+{
+}
diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index 9bfddc1..e1be224 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -411,6 +411,11 @@ found_container:
          goto err_listener_register;
      }

+ ret = vfio_cpr_register_container(bcontainer, errp);

+    if (ret) {
+        goto err_listener_register;
+    }
+
      /*
       * TODO: examine RAM_BLOCK_DISCARD stuff, should we do group level
       * for discarding incompatibility check as well?
@@ -461,6 +466,7 @@ static void iommufd_cdev_detach(VFIODevice *vbasedev)
          iommufd_cdev_ram_block_discard_disable(false);
      }

+ vfio_cpr_unregister_container(bcontainer);

      iommufd_cdev_detach_container(vbasedev, container);
      iommufd_cdev_container_destroy(container);
      vfio_put_address_space(space);
diff --git a/hw/vfio/meson.build b/hw/vfio/meson.build
index bb98493..bba776f 100644
--- a/hw/vfio/meson.build
+++ b/hw/vfio/meson.build
@@ -5,6 +5,7 @@ vfio_ss.add(files(
    'container-base.c',
    'container.c',
    'migration.c',
+  'cpr.c',
  ))
  vfio_ss.add(when: 'CONFIG_PSERIES', if_true: files('spapr.c'))
  vfio_ss.add(when: 'CONFIG_IOMMUFD', if_true: files(
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index 4a6c262..b9da6c0 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -205,6 +205,9 @@ void vfio_detach_device(VFIODevice *vbasedev);
  int vfio_kvm_device_add_fd(int fd, Error **errp);
  int vfio_kvm_device_del_fd(int fd, Error **errp);

+int vfio_cpr_register_container(VFIOContainerBase *bcontainer, Error **errp);

Should we return bool since we have an errp ? the returned value
is not an errno AFAICT.

Anyhow,

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

Thanks,

C.

+void vfio_cpr_unregister_container(VFIOContainerBase *bcontainer);
+
  extern const MemoryRegionOps vfio_region_ops;
  typedef QLIST_HEAD(VFIOGroupList, VFIOGroup) VFIOGroupList;
  typedef QLIST_HEAD(VFIODeviceList, VFIODevice) VFIODeviceList;