Re: [PATCH V5 35/38] vfio/iommufd: change process

[Steven Sistare]

On 6/25/2025 7:40 AM, Duan, Zhenzhong wrote:

-----Original Message-----
From: Steve Sistare <steven.sist...@oracle.com>
Subject: [PATCH V5 35/38] vfio/iommufd: change process

Finish CPR by change the owning process of the iommufd device in
post load.

Signed-off-by: Steve Sistare <steven.sist...@oracle.com>
---
hw/vfio/cpr-iommufd.c | 30 ++++++++++++++++++++++++++++++
1 file changed, 30 insertions(+)

diff --git a/hw/vfio/cpr-iommufd.c b/hw/vfio/cpr-iommufd.c
index 152a661..a9e3f68 100644
--- a/hw/vfio/cpr-iommufd.c
+++ b/hw/vfio/cpr-iommufd.c
@@ -110,10 +110,40 @@ static bool vfio_cpr_supported(IOMMUFDBackend *be,
Error **errp)
     return true;
}

+static int iommufd_cpr_pre_save(void *opaque)
+{
+    IOMMUFDBackend *be = opaque;
+    Error *local_err = NULL;
+
+    /*
+     * The process has not changed yet, but proactively call the ioctl,
+     * and it will fail if any DMA mappings are not supported.
+     */
+    if (!iommufd_change_process(be, &local_err)) {

I'm confused when to call iommufd_change_process_capable and when to call 
iommufd_change_process, could you clarify?

Strictly speaking, we do not need iommufd_change_process_capable, because we 
always
try iommufd_change_process and recover on failure.  But, 
iommufd_change_process_capable
allows us to install a migration blocker, and fail with a blocker error, which 
is considered
more user friendly for migration.

+        error_report_err(local_err);
+        return -1;
+    }
+    return 0;
+}
+
+static int iommufd_cpr_post_load(void *opaque, int version_id)
+{
+     IOMMUFDBackend *be = opaque;
+     Error *local_err = NULL;
+
+     if (!iommufd_change_process(be, &local_err)) {
+        error_report_err(local_err);
+        return -1;
+     }
+     return 0;
+}
+
static const VMStateDescription iommufd_cpr_vmstate = {
     .name = "iommufd",
     .version_id = 0,
     .minimum_version_id = 0,
+    .pre_save = iommufd_cpr_pre_save,
+    .post_load = iommufd_cpr_post_load,

Do we need LOW priority?

No.  iommufd_cpr_post_load only calls iommufd_change_process, which acts upon
mappings that are already known to the kernel, independently of vmstate.

- Steve

     .needed = cpr_incoming_needed,
     .fields = (VMStateField[]) {
         VMSTATE_END_OF_LIST()
--
1.8.3.1