Hi Hari, On Mon, 2013-09-02 at 02:24 +0000, Shankar, Hari wrote: > Patch Description: The patch is for Intel IOMMU driver. It forces IOMMU TLB > flush for the particular domain and return correct unmap size when > intel_iommu_unmap() is called
"Patch Description:" is unnecessary. Look at other commits that have touched this file to get an idea how to format the subject and commit log: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/log/drivers/iommu/intel-iommu.c Sign-off should go here, not below the patch. Subject should include a subsystem, such as "iommu/intel:" or "intel-iommu:". Also a good idea to provide some justification why this is necessary for stable (unmaps from userspace through vfio results in coherency issue...) > The patch is generated on Linux kernel version 3.6.11 This should be noted, but not part of the commit message. See section 15 of SubmittingPatches and note the additional comments between the "---" marker and the beginning of the patch. > --- linux/drivers/iommu/intel-iommu.c.orig 2013-09-01 10:10:14.723958000 > -0700 > +++ linux/drivers/iommu/intel-iommu.c 2013-09-01 18:22:58.497578000 -0700 > @@ -4060,14 +4060,31 @@ static size_t intel_iommu_unmap(struct i Tabs seem to have been clobbered in your patch, scripts/checkpatch.pl will note this for you. > { > struct dmar_domain *dmar_domain = domain->priv; > int order; > + struct intel_iommu *iommu; > + unsigned long start_pfn, last_pfn; > + unsigned int npages; > + int iommu_id, num, ndomains; > > - order = dma_pte_clear_range(dmar_domain, iova >> VTD_PAGE_SHIFT, > - (iova + size - 1) >> VTD_PAGE_SHIFT); > + start_pfn = iova >> VTD_PAGE_SHIFT; > + last_pfn = (iova + size - 1) >> VTD_PAGE_SHIFT; > + order = dma_pte_clear_range(dmar_domain, start_pfn, last_pfn); > + last_pfn |= (1UL << order) - 1; > + npages = last_pfn - start_pfn + 1; Doesn't order tell us npages more directly? npages = 1UL << order? > + for_each_set_bit(iommu_id, dmar_domain->iommu_bmp, g_num_of_iommus) { > + iommu = g_iommus[iommu_id]; > > - if (dmar_domain->max_addr == iova + size) > + /* > + * find bit position of dmar_domain > + */ > + ndomains = cap_ndoms(iommu->cap); > + for_each_set_bit(num, iommu->domain_ids, ndomains) > + if (iommu->domains[num] == dmar_domain) > + iommu_flush_iotlb_psi(iommu, num, > + start_pfn, npages, 0); This second bitmap search seems unnecessary, I think iommu_flush_iotlb_psi wants dmar_domain->id as the second parameter in all cases, not num. So this could be reduced to : for_each_set_bit(iommu_id, dmar_domain->iommu_bmp, g_num_of_iommus) iommu_flush_iotlb_psi(g_iommus[iommu_id], dmar_domain->id, start_pfn, npages, 0); > + } > + if (dmar_domain->max_addr <= iova + (npages << VTD_PAGE_SHIFT)) > dmar_domain->max_addr = iova; > - > - return PAGE_SIZE << order; > + return npages << VTD_PAGE_SHIFT; > } > > static phys_addr_t intel_iommu_iova_to_phys(struct iommu_domain *domain, > > Signed-off-by: Hari Shankar <[email protected]> > cc: [email protected] Please configure your mailer to drop the html version. Thanks, Alex _______________________________________________ iommu mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/iommu
