On 5/25/2025 10:31 PM, Duan, Zhenzhong wrote:
-----Original Message-----
From: Steven Sistare <steven.sist...@oracle.com>
Subject: Re: [PATCH V3 29/42] backends/iommufd: change process ioctl
On 5/23/2025 10:56 AM, Steven Sistare wrote:
On 5/23/2025 4:56 AM, Duan, Zhenzhong wrote:
-----Original Message-----
From: Steven Sistare <steven.sist...@oracle.com>
Subject: Re: [PATCH V3 29/42] backends/iommufd: change process ioctl
On 5/21/2025 11:19 PM, Duan, Zhenzhong wrote:
-----Original Message-----
From: Steven Sistare <steven.sist...@oracle.com>
Subject: Re: [PATCH V3 29/42] backends/iommufd: change process ioctl
On 5/20/2025 11:11 PM, Duan, Zhenzhong wrote:
-----Original Message-----
From: Steven Sistare <steven.sist...@oracle.com>
Subject: Re: [PATCH V3 29/42] backends/iommufd: change process ioctl
On 5/19/2025 11:51 AM, Steven Sistare wrote:
On 5/16/2025 4:42 AM, Duan, Zhenzhong wrote:
-----Original Message-----
From: Steve Sistare <steven.sist...@oracle.com>
Subject: [PATCH V3 29/42] backends/iommufd: change process ioctl
Define the change process ioctl
Signed-off-by: Steve Sistare <steven.sist...@oracle.com>
---
backends/iommufd.c | 20 ++++++++++++++++++++
backends/trace-events | 1 +
include/system/iommufd.h | 2 ++
3 files changed, 23 insertions(+)
diff --git a/backends/iommufd.c b/backends/iommufd.c
index 5c1958f..6fed1c1 100644
--- a/backends/iommufd.c
+++ b/backends/iommufd.c
@@ -73,6 +73,26 @@ static void
iommufd_backend_class_init(ObjectClass
*oc,
const void *data)
object_class_property_add_str(oc, "fd", NULL,
iommufd_backend_set_fd);
}
+bool iommufd_change_process_capable(IOMMUFDBackend *be)
+{
+ struct iommu_ioas_change_process args = {.size = sizeof(args)};
+
+ return !ioctl(be->fd, IOMMU_IOAS_CHANGE_PROCESS, &args);
+}
+
+bool iommufd_change_process(IOMMUFDBackend *be, Error
**errp)
+{
+ struct iommu_ioas_change_process args = {.size = sizeof(args)};
+ bool ret = !ioctl(be->fd, IOMMU_IOAS_CHANGE_PROCESS,
&args);
This is same ioctl as above check, could it be called more than once
for
same
process?
Yes, and it is a no-op if the process has not changed since the last time
DMA
was mapped.
More questions?
Looks a bit redundant for me, meanwhile if
iommufd_change_process_capable()
is called on target qemu, may it do both checking and change?
I would suggest to define only iommufd_change_process() and comment
that
it's no-op if process not changed...
We need to check if IOMMU_IOAS_CHANGE_PROCESS is allowed before
performing
live update so we can add a blocker and prevent live update cleanly:
vfio_iommufd_cpr_register_container
if !vfio_cpr_supported() // calls iommufd_change_process_capable
migrate_add_blocker_modes()
This reminds me of other questions, is this ioctl() suitable for checking if
cpr-
transfer supported?
If there is vIOMMU, there can be no mapping and process_capable() check
will
pass,
but if memory is not file backed...
Does cpr-transfer support vIOMMU or not?
I don't know, I have not tried your sample args yet, but I will.
With vIOMMU, what entity/interface pins memory for the vfio device?
Oh, I don't mean virtio-iommu, it can be intel-iommu or virtio-iommu for this
issue.
I mean when guest attach device to a DMA domain, there can be no mapping
in that domain initially.
QEMU knows details of all memory backends, why not checking memory
backends directly instead of a system call?
IOMMU_IOAS_CHANGE_PROCESS is relatively new. The ioctl verifies that the
kernel
supports it. And if supported, it also verifies that all dma mappings are
of the file type.
But the dma mappings are dynamic if there is vIOMMU, so checking dma
mappings are checking nothing if there is no mapping in the DMA domain.
Yes, so there are 2 checks:
* at realize -> cpr register time. if cpr can never work because
IOMMU_IOAS_CHANGE_PROCESS is not supported, then adds a blocker.
* at cpr time, in vfio_container_pre_save. refuses to proceed if
iommufd_change_process() fails because non-file mappings are present.
Allows cpr if there are no mappings present.
Let me explain further.
There is a corner case that could bypass above checks. Source qemu starts with
vIOMMU and non-file memory backend, then hotplug VFIO device, if guest
driver doesn't setup any mapping or no guest driver attached, the mapping on
host side can be empty, then above checks will both pass.
That is OK. CPR is allowed in that case and succeeds because
iommufd_change_process
has nothing to do.
However, after CPR, if non-file mappings are added, then the next CPR operation
would be blocked.
- Steve
I'm not sure if that's a case we need to support. If not, feel free to add my
RB.
If my explanation makes sense, any chance of getting an RB for this and the
related patch?
backends/iommufd: change process ioctl
vfio/iommufd: change process
They are not affected by the other changes we have discussed.
- Steve
How about I just add a comment:
bool iommufd_change_process_capable(IOMMUFDBackend *be)
{
/*
* Call IOMMU_IOAS_CHANGE_PROCESS to verify it is a recognized
ioctl.
* This is a no-op if the process has not changed since DMA was
mapped.
*/
- Steve
