>>> On 06.05.16 at 10:54, <quan...@intel.com> wrote:
> -static void intel_iommu_iotlb_flush(struct domain *d, unsigned long gfn,
> unsigned int page_count)
> +static void iommu_flush_iotlb_page(struct domain *d, unsigned long gfn,
> + unsigned int page_count)
The new name suggests just one page. Please use e.g.
iommu_flush_iotlb_pages() instead.
> {
> - __intel_iommu_iotlb_flush(d, gfn, 1, page_count);
> + iommu_flush_iotlb(d, gfn, 1, page_count);
> }
But of course the question is whether having this wrapper is useful in
the first place, the more that ...
> @@ -639,7 +646,7 @@ static void dma_pte_clear_one(struct domain *domain, u64
> addr)
> iommu_flush_cache_entry(pte, sizeof(struct dma_pte));
>
> if ( !this_cpu(iommu_dont_flush_iotlb) )
> - __intel_iommu_iotlb_flush(domain, addr >> PAGE_SHIFT_4K, 1, 1);
> + iommu_flush_iotlb(domain, addr >> PAGE_SHIFT_4K, 1, 1);
... it's being open coded here. IOW if you want to retain the
wrapper, please use it here.
> @@ -1391,13 +1399,19 @@ int domain_context_mapping_one(
> spin_unlock(&iommu->lock);
>
> /* Context entry was previously non-present (with domid 0). */
> - if ( iommu_flush_context_device(iommu, 0, (((u16)bus) << 8) | devfn,
> - DMA_CCMD_MASK_NOBIT, 1) )
> - iommu_flush_write_buffer(iommu);
> - else
> + rc = iommu_flush_context_device(iommu, 0, (((u16)bus) << 8) | devfn,
> + DMA_CCMD_MASK_NOBIT, 1);
> +
> + if ( !rc )
> {
> int flush_dev_iotlb = find_ats_dev_drhd(iommu) ? 1 : 0;
> - iommu_flush_iotlb_dsi(iommu, 0, 1, flush_dev_iotlb);
> + rc = iommu_flush_iotlb_dsi(iommu, 0, 1, flush_dev_iotlb);
Please take the opportunity and add the missing blank line (between
declaration(s) and statement(s) in cases like this.
> + }
> +
> + if ( rc > 0 )
Can iommu_flush_context_device() return a positive value? If so,
the logic is now likely wrong. If not (which is what I assume) I'd
like to suggest adding a respective ASSERT() (even if only to
document the fact). Or alternatively this if() could move into the
immediately preceding one.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
