Re: [PATCH v3 1/7] iommu: Introduce a replace API for device pasid

[Baolu Lu <baolu.lu@linux.intel.com>]

On 9/6/24 12:21 PM, Yi Liu wrote:
> On 2024/8/16 21:02, Jason Gunthorpe wrote:
>> On Fri, Aug 16, 2024 at 05:43:18PM +0800, Yi Liu wrote:
>>> On 2024/7/18 16:27, Tian, Kevin wrote:
>>>>> From: Liu, Yi L <yi.l.liu@intel.com>
>>>>> Sent: Friday, June 28, 2024 5:06 PM
>>>>>
>>>>> @@ -3289,7 +3290,20 @@ static int __iommu_set_group_pasid(struct
>>>>> iommu_domain *domain,
>>>>>
>>>>>            if (device == last_gdev)
>>>>>                break;
>>>>> -        ops->remove_dev_pasid(device->dev, pasid, domain);
>>>>> +        /* If no old domain, undo the succeeded devices/pasid */
>>>>> +        if (!old) {
>>>>> +            ops->remove_dev_pasid(device->dev, pasid, domain);
>>>>> +            continue;
>>>>> +        }
>>>>> +
>>>>> +        /*
>>>>> +         * Rollback the succeeded devices/pasid to the old domain.
>>>>> +         * And it is a driver bug to fail attaching with a previously
>>>>> +         * good domain.
>>>>> +         */
>>>>> +        if (WARN_ON(old->ops->set_dev_pasid(old, device->dev,
>>>>> +                            pasid, domain)))
>>>>> +            ops->remove_dev_pasid(device->dev, pasid, domain);
>>>>
>>>> I wonder whether @remove_dev_pasid() can be replaced by having
>>>> blocking_domain support @set_dev_pasid?
>>>
>>> how about your thought, @Jason?
>>
>> I think we talked about doing that once before, I forget why it was
>> not done. Maybe there was an issue?
>>
>> But it seems worth trying.
> 
> Since remove_dev_pasid() does not return a result, so caller does not
> need to check the result of it. If we want to replace it with the
> blocked_domain->ops->set_dev_pasid(), shall we enforce that the
> set_dev_pasid() op of blocked_domain to be always success. Is it?

Yes. The semantics of blocking domain is that the iommu driver must
ensure successful completion.

Thanks,
baolu