Re: [RFC 2/6] omap: iovmm: generic iommu api migration
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date: 2011-06-07 09:05:06
Also in:
linux-arm-kernel, linux-media, lkml
Hi Ohad, Thanks for the patch. On Friday 03 June 2011 00:27:39 Ohad Ben-Cohen wrote:
Migrate omap's iovmm (virtual memory manager) to the generic iommu api. This brings iovmm a step forward towards being completely non omap-specific (it's still assuming omap's iommu page sizes, and also maintaining state inside omap's internal iommu structure, but it no longer calls omap-specific iommu map/unmap api). Further generalizing of iovmm (or complete removal) should take place together with broader plans of providing a generic virtual memory manager and allocation framework (de-coupled from specific mappers). Signed-off-by: Ohad Ben-Cohen <redacted>
[snip]
quoted hunk ↗ jump to hunk
diff --git a/arch/arm/plat-omap/iovmm.c b/arch/arm/plat-omap/iovmm.c index 51ef43e..80bb2b6 100644 --- a/arch/arm/plat-omap/iovmm.c +++ b/arch/arm/plat-omap/iovmm.c
[snip]
quoted hunk ↗ jump to hunk
@@ -473,22 +475,22 @@ static int map_iovm_area(struct iommu *obj, structiovm_struct *new, u32 pa; int pgsz; size_t bytes; - struct iotlb_entry e; pa = sg_phys(sg); bytes = sg_dma_len(sg); flags &= ~IOVMF_PGSZ_MASK; + pgsz = bytes_to_iopgsz(bytes); if (pgsz < 0) goto err_out; - flags |= pgsz;
pgsz isn't used anymore, you can remove it.
+ + order = get_order(bytes);
Does iommu_map() handle offsets correctly, or does it expect pa to be aligned to an order (or other) boundary ? Same comment for iommu_unmap() in unmap_iovm_area().
quoted hunk ↗ jump to hunk
pr_debug("%s: [%d] %08x %08x(%x)\n", __func__, i, da, pa, bytes); - iotlb_init_entry(&e, da, pa, flags); - err = iopgtable_store_entry(obj, &e); + err = iommu_map(domain, da, pa, order, flags); if (err) goto err_out;@@ -502,9 +504,11 @@ err_out: for_each_sg(sgt->sgl, sg, i, j) { size_t bytes; - bytes = iopgtable_clear_entry(obj, da); + bytes = sg_dma_len(sg);
As Russell pointed out, we should use sg->length instead of sg_dma_length(sg). sg_dma_length(sg) is only valid after the scatter list has been DMA-mapped, which doesn't happen in the iovmm driver. This applies to all sg_dma_len(sg) calls.
+ order = get_order(bytes); - BUG_ON(!iopgsz_ok(bytes)); + /* ignore failures.. we're already handling one */ + iommu_unmap(domain, da, order); da += bytes; }
-- Regards, Laurent Pinchart