Re: [Xen-devel] [PATCH v1 02/10] iommu: Add extra order argument to the IOMMU APIs and platform callbacks

[Oleksandr Tyshchenko <olekstysh@xxxxxxxxx>]

Hi, Jan

On Mon, May 15, 2017 at 10:22 AM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>> On 12.05.17 at 18:25, <olekstysh@xxxxxxxxx> wrote:
>> On Fri, May 12, 2017 at 7:17 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>>> On 12.05.17 at 17:50, <olekstysh@xxxxxxxxx> wrote:
>>>> On Fri, May 12, 2017 at 5:23 PM, Jan Beulich <JBeulich@xxxxxxxx> wrote:
>>>>>>>> On 10.05.17 at 16:03, <olekstysh@xxxxxxxxx> wrote:
>>>>>> @@ -771,6 +773,47 @@ int amd_iommu_unmap_page(struct domain *d, unsigned 
>>>>>> long gfn)
>>>>>>      return 0;
>>>>>>  }
>>>>>>
>>>>>> +/* TODO: Optimize by squashing map_pages/unmap_pages with 
>>>>>> map_page/unmap_page */
>>>>>
>>>>> Looking over the titles of the rest of this series it doesn't look like
>>>>> you're eliminating this TODO later. While I appreciate this not
>>>>> being done in the already large patch, I don't think such a TODO
>>>>> should be left around. If need be (e.g. because you can't test
>>>>> the change), get in touch with the maintainer(s).
>>>> I will drop this TODO everywhere.
>>>
>>> By "drop" you mean "address" or really just "drop"?
>> I meant just drop.
>
> Then I'm sorry, but no, this is not a way to address the comment I've
> made.

Indeed, there was some misunderstanding from my side on this.
Let me elaborate a bit more on this:
1. Yes, this TODO shouldn't be just dropped, but needs to be
addressed, so at least I will have them back in the patch
2. I am not a x86 guy and not familiar with the Intel/AMD IOMMUs, so
it makes me lots of work to do this change
properly, so this is not only the question of testing the code, but rather
having it written.
3. Please correct me if I'm wrong, but these are all *optimizations* which
I am mentioning in that TODO, not something that breaks x86 or affects it
in any way.

That being said, can we postpone implementation of the *optimizations*
in question
and have those as a separate activity?
Or if these *optimizations* must be present in the current patch
series, could you, please, provide me with some hints how
these TODO should be properly implemented?

>
> Jan
>


-- 
Regards,

Oleksandr Tyshchenko

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel