Re: [PATCH v4 2/7] iommu/core: split mapping to page sizes as supported by the hardware
On Sat, Nov 12, 2011 at 11:04 AM, Stepan Moskovchenko step...@codeaurora.org wrote: Yes, the scatterlist is allowed to have different page sizes. But, they are required to have a length that is a multiple of 4K. If a page in the list is An element in scatterlist may not be aligned by 4K but I think IOMMU driver must round it up to 4K if IOMMU API support iommu_map_range(). The first element in scatterlist is often not aligned by 4K if the pages are provided from userspace. Length + offset of the element must be aligned by 4K, though. Thank you, KyongHo. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/6] iommu/core: split mapping to page sizes as supported by the hardware
On Tue, Oct 11, 2011 at 7:49 AM, Ohad Ben-Cohen o...@wizery.com wrote:
int iommu_map(struct iommu_domain *domain, unsigned long iova,
- phys_addr_t paddr, int gfp_order, int prot)
+ phys_addr_t paddr, int size, int prot)
{
Even though it is not realistic that size becomes larger than 1
((sizeof(int) * 8) - 1),
I think the type of size should be size_t.
- size_t size;
+ unsigned long orig_iova = iova;
+ int ret = 0, orig_size = size;
if (unlikely(domain-ops-map == NULL))
return -ENODEV;
- size = PAGE_SIZE gfp_order;
+ /*
+ * both the virtual address and the physical one, as well as
+ * the size of the mapping, must be aligned (at least) to the
+ * size of the smallest page supported by the hardware
+ */
+ if (!IS_ALIGNED(iova | paddr | size, domain-ops-min_pagesz)) {
+ pr_err(unaligned: iova 0x%lx pa 0x%lx size 0x%x min_pagesz
+ 0x%x\n, iova, (unsigned long)paddr,
+ size, domain-ops-min_pagesz);
+ return -EINVAL;
+ }
+
+ pr_debug(map: iova 0x%lx pa 0x%lx size 0x%x\n, iova,
+ (unsigned long)paddr, size);
+
+ while (size) {
+ unsigned long pgsize, addr_merge = iova | paddr;
+ unsigned int pgsize_idx;
+
+ /* Max page size that still fits into 'size' */
+ pgsize_idx = __fls(size);
+
+ /* need to consider alignment requirements ? */
+ if (likely(addr_merge)) {
+ /* Max page size allowed by both iova and paddr */
+ unsigned int align_pgsize_idx = __ffs(addr_merge);
+
+ pgsize_idx = min(pgsize_idx, align_pgsize_idx);
+ }
+
+ /* build a mask of acceptable page sizes */
+ pgsize = (1UL (pgsize_idx + 1)) - 1;
+
+ /* throw away page sizes not supported by the hardware */
+ pgsize = domain-ops-pgsize_bitmap;
+
+ /* make sure we're still sane */
+ BUG_ON(!pgsize);
- BUG_ON(!IS_ALIGNED(iova | paddr, size));
+ /* pick the biggest page */
+ pgsize_idx = __fls(pgsize);
+ pgsize = 1UL pgsize_idx;
- return domain-ops-map(domain, iova, paddr, gfp_order, prot);
+ /* convert index to page order */
+ pgsize_idx -= PAGE_SHIFT;
+
+ pr_debug(mapping: iova 0x%lx pa 0x%lx order %u\n, iova,
+ (unsigned long)paddr, pgsize_idx);
+
+ ret = domain-ops-map(domain, iova, paddr, pgsize_idx, prot);
+ if (ret)
+ break;
+
+ iova += pgsize;
+ paddr += pgsize;
+ size -= pgsize;
+ }
+
+ /* unroll mapping in case something went wrong */
+ if (ret)
+ iommu_unmap(domain, orig_iova, orig_size);
+
domain-ops-map() might return error because a mapping already exists
in the range from iova until iova + size.
Thus, it should be
iommu_unmap(domain, orig_iova, orig_size - size)
Regards,
KyongHo
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/6] iommu/core: split mapping to page sizes as supported by the hardware
On Mon, Oct 3, 2011 at 12:58 AM, Ohad Ben-Cohen o...@wizery.com wrote:
int iommu_map(struct iommu_domain *domain, unsigned long iova,
- phys_addr_t paddr, int gfp_order, int prot)
+ phys_addr_t paddr, size_t size, int prot)
{
- size_t size;
+ int ret = 0;
+
+ /*
+ * both the virtual address and the physical one, as well as
+ * the size of the mapping, must be aligned (at least) to the
+ * size of the smallest page supported by the hardware
+ */
+ if (!IS_ALIGNED(iova | paddr | size, iommu_min_pagesz)) {
+ pr_err(unaligned: iova 0x%lx pa 0x%lx size 0x%lx min_pagesz
+ 0x%x\n, iova, (unsigned long)paddr,
+ (unsigned long)size, iommu_min_pagesz);
+ return -EINVAL;
+ }
- size = 0x1000UL gfp_order;
+ pr_debug(map: iova 0x%lx pa 0x%lx size 0x%lx\n, iova,
+ (unsigned long)paddr, (unsigned long)size);
- BUG_ON(!IS_ALIGNED(iova | paddr, size));
+ while (size) {
+ unsigned long pgsize, addr_merge = iova | paddr;
+ unsigned int pgsize_idx;
- return iommu_ops-map(domain, iova, paddr, gfp_order, prot);
+ /* Max page size that still fits into 'size' */
+ pgsize_idx = __fls(size);
+
+ /* need to consider alignment requirements ? */
+ if (likely(addr_merge)) {
+ /* Max page size allowed by both iova and paddr */
+ unsigned int align_pgsize_idx = __ffs(addr_merge);
+
+ pgsize_idx = min(pgsize_idx, align_pgsize_idx);
+ }
+
+ /* build a mask of acceptable page sizes */
+ pgsize = (1UL (pgsize_idx + 1)) - 1;
+
+ /* throw away page sizes not supported by the hardware */
+ pgsize = iommu_pgsize_bitmap;
+
+ /* pick the biggest page */
+ pgsize_idx = __fls(pgsize);
+ pgsize = 1UL pgsize_idx;
+
+ /* convert index to page order */
+ pgsize_idx -= PAGE_SHIFT;
+
+ pr_debug(mapping: iova 0x%lx pa 0x%lx order %u\n, iova,
+ (unsigned long)paddr, pgsize_idx);
+
+ ret = iommu_ops-map(domain, iova, paddr, pgsize_idx, prot);
+ if (ret)
+ break;
+
+ iova += pgsize;
+ paddr += pgsize;
+ size -= pgsize;
+ }
+
+ return ret;
}
EXPORT_SYMBOL_GPL(iommu_map);
Do not we need to unmap all intermediate mappings if iommu_map() is failed?
I think iommu_map() must map the entire given area if it successes.
Otherwise, it should map nothing.
I think it can be simply done with calling iommu_unmap()
with Joerg's previous suggestion about iommu_unmap().
Regards,
KyongHo.
--
To unsubscribe from this list: send the line unsubscribe linux-omap in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 7/7] iommu/core: split mapping to page sizes as supported by the hardware
Hi Ohad, On Wed, Sep 7, 2011 at 6:16 PM, Ohad Ben-Cohen o...@wizery.com wrote: Hmm this sounds a bit like a red herring to me; optimization of the :) I agree. sorry. map function is not the main subject here. Especially not when we're discussing mapping of large physically contiguous memory regions which do not happen too often. I've got your point but I thought that it is really needed. Another advantage for migrating s5p_iommu_map() over to the subject patch, is that s5p_iommu_map() doesn't support super sections yet. To support it, you'd need to add more code (duplicate another while loop). But if you migrated to the subject patch, then you would only need to flip the 16MB bit when you advertise page size capabilities and then that's it; you're done. I did not implement that. 16MB page is less practical in Linux because Linux kernel is unable to allocated larger physically contiguous memory than 4MB by default. But I also think that it is needed to support 16MB mapping for IO virtualization someday and it is just trivial job. And you pointed correctly that s5p_iommu_map() has duplicate similar codes. Actually, I think your idea is good and does not cause performance degradation. But I wondered if it is really useful. The caller of iommu_map() doesn't say anything about alignments. It just gives it a memory region to map, and expect things to just work. The caller of iommu_map() gives gfp_order that is the size of the physical memory to map. I thought that it also means alignment of the physical memory. Isn't it? Regards, KyongHo -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC 7/7] iommu/core: split mapping to page sizes as supported by the hardware
Hi On Sat, Sep 3, 2011 at 2:32 AM, Ohad Ben-Cohen o...@wizery.com wrote: When mapping a memory region, split it to page sizes as supported by the iommu hardware. Always prefer bigger pages, when possible, in order to reduce the TLB pressure. True. It is important for the peripheral devices that works with IOMMU. This allows a more lenient granularity of mappings: traditionally the IOMMU API took 'order' (of a page) as a mapping size, and directly let the low level iommu drivers handle the mapping. Now that the IOMMU core can split arbitrary memory regions into pages, we can remove this limitation, so users don't have to split those regions by themselves. Please find the following link that I submitted for our IOMMU. https://lkml.org/lkml/2011/7/3/152 s5p_iommu_map/unmap accepts any order of physical address and iova without support of your suggestion if the order is not less than PAGE_SHIFT Of course, an IO virtual memory manager must be careful when it allocates IO virtual memory to lead best performance. But I think IOMMU API must not expect that the caller of iommu_map() knows about the restriction of IOMMU API implementation. Regards, Cho KyongHo. -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] OMAP: iommu flush page table entries from L1 and L2 cache
Hi, Russell. I think we need cache maintenance operations that effect on inner and outer caches at the same time. Even though the DMA APIs are not for cache maintenance but for IO mapping, they are useful for cache maint' because they operate on inner and outer caches. As you know, inner cache of Cortex-A is logical cache and outer cache is physical cache in the programmer's point of view. We need logical address and physical address at the same time to clean or invalidate inner and outer cache. That means we need to translate logical to physical address and it is sometimes not trivial. Finally, the kernel will contain many similar routines that do same thing. On Fri, Apr 15, 2011 at 7:30 AM, Russell King - ARM Linux li...@arm.linux.org.uk wrote: On Thu, Apr 14, 2011 at 04:52:48PM -0500, Fernando Guzman Lugo wrote: From: Ramesh Gupta grgu...@ti.com This patch is to flush the iommu page table entries from L1 and L2 caches using dma_map_single. This also simplifies the implementation by removing the functions flush_iopgd_range/flush_iopte_range. No. This usage is just wrong. If you're going to use the DMA API then unmap it, otherwise the DMA API debugging will go awol. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- To unsubscribe from this list: send the line unsubscribe linux-omap in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
