>-----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.
>
>- 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
>>
