lkml.org 
[lkml]   [2016]   [May]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH 4/5] iommu/mediatek: add support for mtk iommu generation one HW
From
Date
On 09/05/16 09:00, honghui.zhang@mediatek.com wrote:
[...]
> +static void *mtk_iommu_alloc_pgt(struct device *dev, size_t size, gfp_t gfp)
> +{
> + dma_addr_t dma;
> + void *pages = alloc_pages_exact(size, gfp | __GFP_ZERO);
> +
> + if (!pages)
> + return NULL;
> +
> + dma = dma_map_single(dev, pages, size, DMA_TO_DEVICE);
> + if (dma_mapping_error(dev, dma))
> + goto out_free;
> + /*
> + * We depend on the IOMMU being able to work with any physical
> + * address directly, so if the DMA layer suggests otherwise by
> + * translating or truncating them, that bodes very badly...
> + */
> + if (dma != virt_to_phys(pages))
> + goto out_unmap;

Given that you've only got a single table to allocate, and at 4MB it has
a fair chance of failing beyond early boot time, just use
dma_alloc_coherent() - you don't need to care about the dma <-> phys
relationship because you don't have multi-level tables to walk. That
way, you can get rid of all the awkward streaming DMA stuff, and also
benefit from CMA to avoid allocation failures.

> + kmemleak_ignore(pages);
> + return pages;
> +
> +out_unmap:
> + dev_err(dev, "Cannot accommodate DMA translation for IOMMU page tables\n");
> + dma_unmap_single(dev, dma, size, DMA_TO_DEVICE);
> +out_free:
> + free_pages_exact(pages, size);
> + return NULL;
> +
> +}
> +
> +static void mtk_iommu_free_pgt(struct device *dev, void *pages, size_t size)
> +{
> + dma_unmap_single(dev, (dma_addr_t)virt_to_phys(pages),
> + size, DMA_TO_DEVICE);
> + free_pages_exact(pages, size);
> +}
> +
> +static int mtk_iommu_domain_finalise(struct mtk_iommu_data *data)
> +{
> + struct mtk_iommu_domain *dom = data->m4u_dom;
> +
> + spin_lock_init(&dom->pgtlock);
> +
> + dom->pgt_va = mtk_iommu_alloc_pgt(data->dev,
> + dom->pgt_size, GFP_KERNEL);
> + if (!dom->pgt_va)
> + return -ENOMEM;
> +
> + dom->pgt_pa = virt_to_phys(dom->pgt_va);
> +
> + writel(dom->pgt_pa, data->base + REG_MMU_PT_BASE_ADDR);
> +
> + dom->cookie = (void *)data;
> +
> + return 0;
> +}
> +
> +static struct iommu_domain *mtk_iommu_domain_alloc(unsigned type)
> +{
> + struct mtk_iommu_domain *dom;
> +
> + if (type != IOMMU_DOMAIN_UNMANAGED)
> + return NULL;
> +
> + dom = kzalloc(sizeof(*dom), GFP_KERNEL);
> + if (!dom)
> + return NULL;
> +
> + /*
> + * MTK m4u support 4GB iova address space, and oly support 4K page
> + * mapping. So the pagetable size should be exactly as 4M.
> + */
> + dom->pgt_size = SZ_4M;

If the table size is fixed, then why bother having a variable at all?

> + return &dom->domain;
> +}
> +
> +static void mtk_iommu_domain_free(struct iommu_domain *domain)
> +{
> + kfree(to_mtk_domain(domain));
> +}
> +

[...]

> +static int mtk_iommu_map(struct iommu_domain *domain, unsigned long iova,
> + phys_addr_t paddr, size_t size, int prot)
> +{
> + struct mtk_iommu_domain *dom = to_mtk_domain(domain);
> + struct mtk_iommu_data *data = dom->cookie;
> + unsigned int page_num = size >> MTK_IOMMU_PAGE_SHIFT;

Since you only advertise a single page size, this will always be 1, so
you could either get rid of the loop here...

> + unsigned long flags;
> + unsigned int i;
> + u32 *pgt_base_iova;
> + u32 pabase = (u32)paddr;
> + int map_size = 0;
> +
> + spin_lock_irqsave(&dom->pgtlock, flags);
> + pgt_base_iova = dom->pgt_va + (iova >> MTK_IOMMU_PAGE_SHIFT);
> + for (i = 0; i < page_num; i++) {
> + pgt_base_iova[i] = pabase | F_DESC_VALID | F_DESC_NONSEC;
> + pabase += MTK_IOMMU_PAGE_SIZE;
> + map_size += MTK_IOMMU_PAGE_SIZE;
> + }
> + dma_sync_single_for_device(data->dev,
> + dom->pgt_pa + (iova >> MTK_IOMMU_PAGE_SHIFT),
> + (size >> MTK_IOMMU_PAGE_SHIFT) * sizeof(u32),
> + DMA_TO_DEVICE);
> + spin_unlock_irqrestore(&dom->pgtlock, flags);
> +
> + mtk_iommu_tlb_flush_range(data, iova, size);
> +
> + return map_size;
> +}

[...]

> +static struct iommu_ops mtk_iommu_ops = {
> + .domain_alloc = mtk_iommu_domain_alloc,
> + .domain_free = mtk_iommu_domain_free,
> + .attach_dev = mtk_iommu_attach_device,
> + .detach_dev = mtk_iommu_detach_device,
> + .map = mtk_iommu_map,
> + .unmap = mtk_iommu_unmap,
> + .map_sg = default_iommu_map_sg,
> + .iova_to_phys = mtk_iommu_iova_to_phys,
> + .add_device = mtk_iommu_add_device,
> + .remove_device = mtk_iommu_remove_device,
> + .device_group = mtk_iommu_device_group,
> + .pgsize_bitmap = MTK_IOMMU_PAGE_SIZE,
> +};

...or perhaps advertise .pgsize_bitmap = ~0UL << MTK_IOMMU_PAGE_SHIFT
here, so you actually can handle multiple entries at once for larger
mappings - given how simple the page table format is that doesn't seem
too unreasonable, especially since it should give you a big efficiency
win in terms of TLB maintenance.

Robin.

\
 
 \ /
  Last update: 2016-05-10 12:41    [W:0.137 / U:0.028 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site