Re: [Qemu-devel] [PATCH RFC v3 11/14] intel_iommu: provide its own replay() callback

[Jason Wang <jasowang@redhat.com>]

On 2017年01月16日 16:06, Peter Xu wrote:
> On Mon, Jan 16, 2017 at 04:03:22PM +0800, Jason Wang wrote:
>
> [...]
>
>>>>> Though I haven't tested with huge pages yet, but this patch should
>>>>> both solve above issue? I don't know whether you went over the page
>>>>> walk logic - it should both support huge page, and it will skip
>>>>> unmapped iova range (at least that's my goal to have this patch). In
>>>>> that case, looks like this patch is solving the same problem? :)
>>>>> (though without introducing iova_iterate() interface)
>>>>>
>>>>> Please correct me if I misunderstood it.
>>>> Kind of :) I'm fine with this patch, but just want:
>>>>
>>>> - reuse most of the codes in the patch
>>>> - current memory_region_iommu_replay() logic
>>>>
>>>> So what I'm suggesting is a just slight change of API which can let caller
>>>> decide it need to do with each range of iova. So it could be reused for
>>>> other things except for replaying.
>>>>
>>>> But if you like to keep this patch as is, I don't object it.
>>> I see. Then I can understand your mean here. I had the same thought
>>> before, that's why I exposed the vtd_page_walk with a hook. If you
>>> check the page_walk function comment:
>>>
>>> /**
>>>   * vtd_page_walk - walk specific IOVA range, and call the hook
>>>   *
>>>   * @ce: context entry to walk upon
>>>   * @start: IOVA address to start the walk
>>>   * @end: IOVA range end address (start <= addr < end)
>>>   * @hook_fn: the hook that to be called for each detected area
>>>   * @private: private data for the hook function
>>>   */
>>>
>>> So I didn't implement the notification in page_walk at all - but in
>>> the hook_fn. If any caller that is interested in doing something else
>>> rather than the notification, we can just simply export the page walk
>>> interface and provide his/her own "hook_fn", then it'll be triggered
>>> for each valid page (no matter a huge/small one).
>>>
>>> If we can have a more general interface in the future - no matter
>>> whether we call it iova_iterate() or something else (I'll prefer the
>>> hooker way to do it, so maybe a common page walker with a hook
>>> function), we can do it simply (at least for Intel platform) based on
>>> this vtd_page_walk thing.
>>>
>>> Thanks,
>>>
>>> -- peterx
>> Yes but the problem is hook_fn is only visible inside intel iommu code.
> Right.
>
> Btw, do we have existing issue that can leverage this interface
> besides replay?
>
> -- peterx

Seems not, so I'm fine with current code, just want to show the 
possibility for it to be reused in the future.

Thanks