Re: [PATCH v3 12/24] powerpc/iommu/powernv: Release replaced TCE

On 02/04/2015 05:08 PM, Paul Mackerras wrote:

On Thu, Jan 29, 2015 at 08:21:53PM +1100, Alexey Kardashevskiy wrote:

quoted

At the moment writing new TCE value to the IOMMU table fails with EBUSY
if there is a valid entry already. However PAPR specification allows
the guest to write new TCE value without clearing it first.

Another problem this patch is addressing is the use of pool locks for
external IOMMU users such as VFIO. The pool locks are to protect
DMA page allocator rather than entries and since the host kernel does
not control what pages are in use, there is no point in pool locks and
exchange()+put_page(oldtce) is sufficient to avoid possible races.

This adds an exchange() callback to iommu_table_ops which does the same
thing as set() plus it returns replaced TCE(s) so the caller can release
the pages afterwards.

This implements exchange() for IODA2 only. This adds a requirement
for a platform to have exchange() implemented so from now on IODA2 is
the only supported PHB for VFIO-SPAPR.

This replaces iommu_tce_build() and iommu_clear_tce() with
a single iommu_tce_xchg().

[snip]

quoted

@@ -294,8 +303,9 @@ static long tce_iommu_build(struct tce_container *container,
 
 		hva = (unsigned long) page_address(page) +
 			(tce & IOMMU_PAGE_MASK(tbl) & ~PAGE_MASK);
+		oldtce = 0;
 
-		ret = iommu_tce_build(tbl, entry + 1, hva, direction);
+		ret = iommu_tce_xchg(tbl, entry + i, hva, &oldtce, direction);

Is the change from entry + 1 to entry + i here an actual bug fix?
If so please mention it in the patch description.

This patch added the bug:
[PATCH v3 01/24] vfio: powerpc/spapr: Move page pinning from arch code to
VFIO IOMMU driver

Will fix in the next try.


-- 
Alexey