On 7/2/2025 9:46 AM, Duan, Zhenzhong wrote:
-----Original Message-----
From: Steven Sistare <steven.sist...@oracle.com>
Subject: Re: [PATCH V5 35/38] vfio/iommufd: change process
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.
Though they are same effect, iommufd_change_process_capable still looks better than iommufd_change_process in pre_save(), because we want to check instead of really changing anything here.
Yes, I will make that change.
- Steve
Other than that, this patch looks good to me.
Reviewed-by: Zhenzhong Duan <zhenzhong.d...@intel.com>
Unrelated to this patch, after further thinking, I suspect that blocker is
never installed. Because vfio_iommufd_cpr_register_iommufd() is called for
first VFIO device and before memory listener is installed.
iommufd_change_process_capable() will always return true.
+ 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.
OK.
