On 5/16/2025 6:06 AM, Duan, Zhenzhong wrote:
-----Original Message-----
From: Steve Sistare <steven.sist...@oracle.com>
Subject: [PATCH V3 36/42] vfio/iommufd: preserve descriptors
Save the iommu and vfio device fd in CPR state when it is created.
After CPR, the fd number is found in CPR state and reused. Remember
the reused status for subsequent patches. The reused status is cleared
when vmstate load finishes.
Signed-off-by: Steve Sistare <steven.sist...@oracle.com>
---
backends/iommufd.c | 19 ++++++++++---------
hw/vfio/cpr-iommufd.c | 16 ++++++++++++++++
hw/vfio/device.c | 10 ++--------
hw/vfio/iommufd.c | 13 +++++++++++--
include/system/iommufd.h | 1 +
5 files changed, 40 insertions(+), 19 deletions(-)
diff --git a/backends/iommufd.c b/backends/iommufd.c
index 6fed1c1..492747c 100644
--- a/backends/iommufd.c
+++ b/backends/iommufd.c
@@ -16,12 +16,18 @@
#include "qemu/module.h"
#include "qom/object_interfaces.h"
#include "qemu/error-report.h"
+#include "migration/cpr.h"
#include "monitor/monitor.h"
#include "trace.h"
#include "hw/vfio/vfio-device.h"
#include <sys/ioctl.h>
#include <linux/iommufd.h>
+static const char *iommufd_fd_name(IOMMUFDBackend *be)
+{
+ return object_get_canonical_path_component(OBJECT(be));
+}
+
static void iommufd_backend_init(Object *obj)
{
IOMMUFDBackend *be = IOMMUFD_BACKEND(obj);
@@ -47,9 +53,8 @@ static void iommufd_backend_set_fd(Object *obj, const
char *str, Error **errp)
IOMMUFDBackend *be = IOMMUFD_BACKEND(obj);
int fd = -1;
- fd = monitor_fd_param(monitor_cur(), str, errp);
+ fd = cpr_get_fd_param(iommufd_fd_name(be), str, 0, &be->cpr_reused, errp);
if (fd == -1) {
- error_prepend(errp, "Could not parse remote object fd %s:", str);
return;
}
be->fd = fd;
@@ -95,14 +100,9 @@ bool iommufd_change_process(IOMMUFDBackend *be,
Error **errp)
bool iommufd_backend_connect(IOMMUFDBackend *be, Error **errp)
{
- int fd;
-
if (be->owned && !be->users) {
- fd = qemu_open("/dev/iommu", O_RDWR, errp);
- if (fd < 0) {
- return false;
- }
- be->fd = fd;
+ be->fd = cpr_open_fd("/dev/iommu", O_RDWR, iommufd_fd_name(be), 0,
+ &be->cpr_reused, errp);
Need to check error before assign to be->fd.
will do.
}
be->users++;
@@ -121,6 +121,7 @@ void iommufd_backend_disconnect(IOMMUFDBackend
*be)
be->fd = -1;
}
out:
+ cpr_delete_fd(iommufd_fd_name(be), 0);
trace_iommufd_backend_disconnect(be->fd, be->users);
}
diff --git a/hw/vfio/cpr-iommufd.c b/hw/vfio/cpr-iommufd.c
index 46f2006..b760bd3 100644
--- a/hw/vfio/cpr-iommufd.c
+++ b/hw/vfio/cpr-iommufd.c
@@ -8,6 +8,7 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
#include "hw/vfio/vfio-cpr.h"
+#include "hw/vfio/vfio-device.h"
#include "migration/blocker.h"
#include "migration/cpr.h"
#include "migration/migration.h"
@@ -25,10 +26,25 @@ static bool vfio_cpr_supported(VFIOIOMMUFDContainer
*container, Error **errp)
return true;
}
+static int vfio_container_post_load(void *opaque, int version_id)
+{
+ VFIOIOMMUFDContainer *container = opaque;
+ VFIOContainerBase *bcontainer = &container->bcontainer;
+ VFIODevice *vbasedev;
+
+ QLIST_FOREACH(vbasedev, &bcontainer->device_list, container_next) {
+ vbasedev->cpr.reused = false;
+ }
+ container->be->cpr_reused = false;
It's strange to set iommufd and vfio device's reused in container's post load,
Maybe better to do it in their own post load handler?
vfio_container_post_load has MIG_PRI_LOW so it is called last, which guarantees
that be->cpr_reused remains true while all devices are loaded. This is required
so that we supress dma_map calls during device load processing:
iommufd_backend_map_file_dma()
if (be->cpr_reused)
return 0;
"vbasedev->cpr.reused = false" could be moved to vfio_device_post_load.
I put it here to be future proof -- al reused flags are cleared together,
at the end of post_load, and to be consistent with
cpr-legacy.c:vfio_container_post_load
+
+ return 0;
+}
+
static const VMStateDescription vfio_container_vmstate = {
.name = "vfio-iommufd-container",
.version_id = 0,
.minimum_version_id = 0,
+ .post_load = vfio_container_post_load,
.needed = cpr_needed_for_reuse,
.fields = (VMStateField[]) {
VMSTATE_END_OF_LIST()
diff --git a/hw/vfio/device.c b/hw/vfio/device.c
index 8e9de68..02f384e 100644
--- a/hw/vfio/device.c
+++ b/hw/vfio/device.c
@@ -312,14 +312,8 @@ bool vfio_device_get_name(VFIODevice *vbasedev,
Error **errp)
void vfio_device_set_fd(VFIODevice *vbasedev, const char *str, Error **errp)
{
- ERRP_GUARD();
- int fd = monitor_fd_param(monitor_cur(), str, errp);
-
- if (fd < 0) {
- error_prepend(errp, "Could not parse remote object fd %s:", str);
- return;
- }
- vbasedev->fd = fd;
+ vbasedev->fd = cpr_get_fd_param(vbasedev->dev->id, str, 0,
+ &vbasedev->cpr.reused, errp);
Same here.
Do you mean, "need to check error"?
If so, no need. The new function definition is:
void vfio_device_set_fd(VFIODevice *vbasedev, const char *str, Error **errp)
{
vbasedev->fd = cpr_get_fd_param(vbasedev->dev->id, str, 0,
&vbasedev->cpr.reused, errp);
}
cpr_get_fd_param() returns -1 on error and sets errp.
}
static VFIODeviceIOOps vfio_device_io_ops_ioctl;
diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
index dabb948..046f601 100644
--- a/hw/vfio/iommufd.c
+++ b/hw/vfio/iommufd.c
@@ -26,6 +26,7 @@
#include "qemu/cutils.h"
#include "qemu/chardev_open.h"
#include "migration/blocker.h"
+#include "migration/cpr.h"
#include "pci.h"
#include "vfio-iommufd.h"
#include "vfio-helpers.h"
@@ -530,13 +531,18 @@ static bool iommufd_cdev_attach(const char *name,
VFIODevice *vbasedev,
VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_IOMMUFD));
if (vbasedev->fd < 0) {
- devfd = iommufd_cdev_getfd(vbasedev->sysfsdev, errp);
+ devfd = cpr_find_fd(vbasedev->name, 0);
+ vbasedev->cpr.reused = (devfd >= 0);
+ if (!vbasedev->cpr.reused) {
+ devfd = iommufd_cdev_getfd(vbasedev->sysfsdev, errp);
+ }
if (devfd < 0) {
return false;
}
vbasedev->fd = devfd;
} else {
devfd = vbasedev->fd;
+ /* reused was set in iommufd_backend_set_fd */
Should be vfio_device_set_fd
yes.
- Steve
}
if (!iommufd_cdev_connect_and_bind(vbasedev, errp)) {
@@ -634,7 +640,9 @@ found_container:
vfio_device_prepare(vbasedev, bcontainer, &dev_info);
vfio_iommufd_cpr_register_device(vbasedev);
-
+ if (!vbasedev->cpr.reused) {
+ cpr_save_fd(vbasedev->name, 0, vbasedev->fd);
+ }
trace_iommufd_cdev_device_info(vbasedev->name, devfd, vbasedev-
num_irqs,
vbasedev->num_regions, vbasedev->flags);
return true;
@@ -673,6 +681,7 @@ static void iommufd_cdev_detach(VFIODevice *vbasedev)
migrate_del_blocker(&vbasedev->cpr.id_blocker);
vfio_iommufd_cpr_unregister_device(vbasedev);
+ cpr_delete_fd(vbasedev->name, 0);
iommufd_cdev_unbind_and_disconnect(vbasedev);
close(vbasedev->fd);
}
diff --git a/include/system/iommufd.h b/include/system/iommufd.h
index db9ed53..5c17abd 100644
--- a/include/system/iommufd.h
+++ b/include/system/iommufd.h
@@ -32,6 +32,7 @@ struct IOMMUFDBackend {
/*< protected >*/
int fd; /* /dev/iommu file descriptor */
bool owned; /* is the /dev/iommu opened internally */
+ bool cpr_reused; /* fd is reused after CPR */
uint32_t users;
/*< public >*/
--
1.8.3.1
