| Message ID | E959C4978C3B6342920538CF579893F001C608DE@SHSMSX104.ccr.corp.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, 2013-10-25 at 11:21 +0000, Wu, Feng wrote: > Actual unmapped size should be returned by intel_iommu_unmap(), because > iommu_map() which calls this function depends on the real unmapped size. > However, in the current logic, the return value of intel_iommu_unmap() > is far smaller than the actual unmapped size, which leads a lot of > redundant calls to intel_iommu_unmap() in iommu_map(). Since dma_pte_clear_range() > can always unmap the space from "start_pfn" to "last_pfn" successfully, > it is okay to return "size" for intel_iommu_unmap(). This is an intel-iommu patch not a KVM patch so it should go to the iommu list and copy the maintainer. Secondly, this seems wrong to me. Neither KVM nor VFIO track the size of individual mappings, so when we unmap a page that was previously mapped as a large page, we we try to unmap 4K and test the return value to see what was actually unmapped. That may be 2M, 1G, etc. With this change we'll try to unmap each 4K page within a larger mapping, even if the first mapping actually unmapped it already. Thanks, Alex > Signed-off-by: Feng Wu <feng.wu@intel.com> > --- > drivers/iommu/intel-iommu.c | 5 ++--- > 1 files changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c > index 15e9b57..bb795d5 100644 > --- a/drivers/iommu/intel-iommu.c > +++ b/drivers/iommu/intel-iommu.c > @@ -4113,15 +4113,14 @@ static size_t intel_iommu_unmap(struct iommu_domain *domain, > unsigned long iova, size_t size) > { > struct dmar_domain *dmar_domain = domain->priv; > - int order; > > - order = dma_pte_clear_range(dmar_domain, iova >> VTD_PAGE_SHIFT, > + dma_pte_clear_range(dmar_domain, iova >> VTD_PAGE_SHIFT, > (iova + size - 1) >> VTD_PAGE_SHIFT); > > if (dmar_domain->max_addr == iova + size) > dmar_domain->max_addr = iova; > > - return PAGE_SIZE << order; > + return size; > } > > static phys_addr_t intel_iommu_iova_to_phys(struct iommu_domain *domain, > -- > 1.7.1 > > BTW: Here is the only place where the return value of dma_pte_clear_range() is used, if we don't use it here neither, maybe we can make it a void function. > -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/iommu/intel-iommu.c b/drivers/iommu/intel-iommu.c index 15e9b57..bb795d5 100644 --- a/drivers/iommu/intel-iommu.c +++ b/drivers/iommu/intel-iommu.c @@ -4113,15 +4113,14 @@ static size_t intel_iommu_unmap(struct iommu_domain *domain, unsigned long iova, size_t size) { struct dmar_domain *dmar_domain = domain->priv; - int order; - order = dma_pte_clear_range(dmar_domain, iova >> VTD_PAGE_SHIFT, + dma_pte_clear_range(dmar_domain, iova >> VTD_PAGE_SHIFT, (iova + size - 1) >> VTD_PAGE_SHIFT); if (dmar_domain->max_addr == iova + size) dmar_domain->max_addr = iova; - return PAGE_SIZE << order; + return size; } static phys_addr_t intel_iommu_iova_to_phys(struct iommu_domain *domain,
Actual unmapped size should be returned by intel_iommu_unmap(), because iommu_map() which calls this function depends on the real unmapped size. However, in the current logic, the return value of intel_iommu_unmap() is far smaller than the actual unmapped size, which leads a lot of redundant calls to intel_iommu_unmap() in iommu_map(). Since dma_pte_clear_range() can always unmap the space from "start_pfn" to "last_pfn" successfully, it is okay to return "size" for intel_iommu_unmap(). Signed-off-by: Feng Wu <feng.wu@intel.com> --- drivers/iommu/intel-iommu.c | 5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) -- 1.7.1 BTW: Here is the only place where the return value of dma_pte_clear_range() is used, if we don't use it here neither, maybe we can make it a void function.