Re: [Xen-devel] [PATCH 2/6] iommu: Introduce iommu_flush and iommu_flush_all.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 08.11.11 at 11:25, Jean Guyader <jean.guyader@xxxxxxxxxxxxx> wrote:
> On 08/11 08:24, Jan Beulich wrote:
>> >>> On 07.11.11 at 18:06, Jean Guyader <jean.guyader@xxxxxxxxxxxxx> wrote:
>> > On 07/11 04:42, Jan Beulich wrote:
>> >> >>> On 07.11.11 at 16:16, Jean Guyader <jean.guyader@xxxxxxxxxxxxx> wrote:
>> >> 
>> >> > Signed-off-by: Jean Guyader <jean.guyader@xxxxxxxxxxxxx>
>> >> > ---
>> >> >  xen/drivers/passthrough/iommu.c     |   20 ++++++++++++++++++++
>> >> >  xen/drivers/passthrough/vtd/iommu.c |   12 ++++++++++++
>> >> >  xen/include/xen/iommu.h             |    5 +++++
>> >> >  3 files changed, 37 insertions(+), 0 deletions(-)
>> >> 
>> >> In iommu_iotlb_flush() you check whether the to-be-called function
>> >> pointer is NULL, whereas in iommu_iotlb_flush_all() you don't. I
>> >> actually think the second behavior is the correct one, but that
>> >> implies that you need to also implement respective AMD IOMMU
>> >> functions.
>> >> 
>> > 
>> > Yes, It's an error on my part. I've updated the patch to check
>> > for the present of iotlb_flush_all before I call it now.
>> 
>> But as said, I don't think this is the right solution: How can it be
>> correct on non-Intel hardware to have these functions simply do
>> nothing?
>> 
> 
> These functions are only here to counter balance the dont_flush_iotlb_flag.
> If they don't acknowlage this flag doing nothing is the right thing to do.

Ah, okay, I overlooked this aspect. Might be worth a comment in
the header file.

Jan

> That said I can probably implement a naive version for the AMD iommu
> that will use amd_iommu_flush_pages once or in a loop.
> 
> Obviously I will remove the check for the non existing callback if I do
> that as it become mandatory.
> 
> Jean




_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel