>-----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?
QEMU knows details of all memory backends, why not checking memory backends
directly instead of a system call?
>
>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
