Re: [PATCH] iommu: add preemption support to iommu_{un,}map()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 28.06.2022 15:08, Roger Pau Monné wrote:
> On Thu, Jun 23, 2022 at 11:49:00AM +0200, Jan Beulich wrote:
>> On 10.06.2022 10:32, Roger Pau Monne wrote:
>>> --- a/xen/include/xen/iommu.h
>>> +++ b/xen/include/xen/iommu.h
>>> @@ -155,10 +155,10 @@ enum
>>>  
>>>  int __must_check iommu_map(struct domain *d, dfn_t dfn, mfn_t mfn,
>>>                             unsigned long page_count, unsigned int flags,
>>> -                           unsigned int *flush_flags);
>>> +                           unsigned int *flush_flags, unsigned long *done);
>>>  int __must_check iommu_unmap(struct domain *d, dfn_t dfn,
>>>                               unsigned long page_count,
>>> -                             unsigned int *flush_flags);
>>> +                             unsigned int *flush_flags, unsigned long 
>>> *done);
>>
>> While I'm okay with adding a 6th parameter to iommu_unmap(), I'm afraid
>> I don't really like adding a 7th one to iommu_map(). I'd instead be
>> inclined to overload the return values of both functions, with positive
>> values indicating "partially done, this many completed".
> 
> We need to be careful then so that the returned value is not
> overflowed by the input count of pages, which is of type unsigned
> long.

Of course.

>> The 6th
>> parameter of iommu_unmap() would then be a "flags" one, with one bit
>> identifying whether preemption is to be checked for. Thoughts?
> 
> Seems fine, but we migth want to do the same for iommu_unmap() in
> order to keep a consistent interface between both?  Not strictly
> required, but it's always better in order to avoid mistakes.

That was the plan - both functions would then have a "flags" parameter,
replacing unmap()'s order one.

> Are you OK with doing the changes and incorporating into your series?

Of course. I was merely waiting with doing the integration until having
feedback from you on my questions / remarks. Thanks for that.

Jan