linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: "Laura Abbott" <lauraa@codeaurora.org>
To: Marek Szyprowski <m.szyprowski@samsung.com>
Cc: linux-arm-kernel@lists.infradead.org,
	linaro-mm-sig@lists.linaro.org, linux-mm@kvack.org,
	linux-arch@vger.kernel.org, Arnd Bergmann <arnd@arndb.de>,
	Joerg Roedel <joro@8bytes.org>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Andrzej Pietrasiewicz <andrzej.p@samsung.com>,
	Russell King - ARM Linux <linux@arm.linux.org.uk>,
	Chunsang Jeong <chunsang.jeong@linaro.org>
Subject: Re: [Linaro-mm-sig] [PATCH 1/2] ARM: initial proof-of-concept IOMMU mapper for DMA-mapping
Date: Thu, 8 Sep 2011 09:41:46 -0700 (PDT)	[thread overview]
Message-ID: <594816116217195c28de13accaf1f9f2.squirrel@www.codeaurora.org> (raw)
In-Reply-To: <1314971786-15140-2-git-send-email-m.szyprowski@samsung.com>


Hi, a few comments
On Fri, September 2, 2011 6:56 am, Marek Szyprowski wrote:
...
> +
> +struct dma_iommu_mapping {
> +	/* iommu specific data */
> +	struct iommu_domain	*domain;
> +
> +	void			*bitmap;

In the earlier version of this patch you had this as a genpool instead of
just doing the bitmaps manually. Is there a reason genpool can't be used
to get the iova addresses?
> +	size_t			bits;
> +	unsigned int		order;
> +	dma_addr_t		base;
> +
> +	struct mutex		lock;
> +};
<snip>
> +int arm_iommu_attach_device(struct device *dev, dma_addr_t base, size_t
> size, int order)
> +{
> +	unsigned int count = (size >> PAGE_SHIFT) - order;
> +	unsigned int bitmap_size = BITS_TO_LONGS(count) * sizeof(long);
> +	struct dma_iommu_mapping *mapping;
> +	int err = -ENOMEM;
> +
> +	mapping = kzalloc(sizeof(struct dma_iommu_mapping), GFP_KERNEL);
> +	if (!mapping)
> +		goto err;
> +
> +	mapping->bitmap = kzalloc(bitmap_size, GFP_KERNEL);
> +	if (!mapping->bitmap)
> +		goto err2;
> +
> +	mapping->base = base;
> +	mapping->bits = bitmap_size;
> +	mapping->order = order;
> +	mutex_init(&mapping->lock);
> +
> +	mapping->domain = iommu_domain_alloc();
> +	if (!mapping->domain)
> +		goto err3;
> +
> +	err = iommu_attach_device(mapping->domain, dev);
> +	if (err != 0)
> +		goto err4;
> +
> +	dev->archdata.mapping = mapping;
> +	set_dma_ops(dev, &iommu_ops);
> +
> +	printk(KERN_INFO "Attached IOMMU controller to %s device.\n",
> dev_name(dev));
> +	return 0;
> +
> +err4:
> +	iommu_domain_free(mapping->domain);
> +err3:
> +	kfree(mapping->bitmap);
> +err2:
> +	kfree(mapping);
> +err:
> +	return -ENOMEM;
> +}
> +EXPORT_SYMBOL(arm_iommu_attach_device);
> +
> +#endif
Attach makes the assumption that each iommu device will exist in a
separate domain. What if multiple devices want to use the same iommu
domain? The msm iommu implementation has many different iommu devices but
many of these will need the same buffer to be mapped in each context so
currently many devices share the same domain. Without this, the same map
call would need to happen for each device, which creates extra map calls
and overhead.
>
> _______________________________________________
> Linaro-mm-sig mailing list
> Linaro-mm-sig@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-mm-sig
>

Laura
-- 
Sent by an employee of the Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.


--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  reply	other threads:[~2011-09-08 16:41 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-09-02 13:56 [RFC 0/2 v2] ARM: DMA-mapping & IOMMU integration Marek Szyprowski
2011-09-02 13:56 ` [PATCH 1/2] ARM: initial proof-of-concept IOMMU mapper for DMA-mapping Marek Szyprowski
2011-09-08 16:41   ` Laura Abbott [this message]
2011-09-21 14:50     ` [Linaro-mm-sig] " Marek Szyprowski
2011-10-10 21:56       ` Krishna Reddy
2011-10-11  8:19         ` Marek Szyprowski
2011-10-11 18:13           ` Krishna Reddy
2011-10-12  1:34             ` Krishna Reddy
2011-10-12  5:49               ` Marek Szyprowski
2011-10-12  7:02                 ` Krishna Reddy
2011-10-13  0:18                   ` Krishna Reddy
2011-10-17 14:07                     ` Marek Szyprowski
2011-09-02 13:56 ` [PATCH 2/2] ARM: Samsung: update/rewrite Samsung SYSMMU (IOMMU) driver Marek Szyprowski
2011-09-05 18:21   ` Ohad Ben-Cohen
2011-09-06 10:27   ` KyongHo Cho
2011-09-06 12:35     ` [Linaro-mm-sig] " Ohad Ben-Cohen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=594816116217195c28de13accaf1f9f2.squirrel@www.codeaurora.org \
    --to=lauraa@codeaurora.org \
    --cc=andrzej.p@samsung.com \
    --cc=arnd@arndb.de \
    --cc=chunsang.jeong@linaro.org \
    --cc=joro@8bytes.org \
    --cc=kyungmin.park@samsung.com \
    --cc=linaro-mm-sig@lists.linaro.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-mm@kvack.org \
    --cc=linux@arm.linux.org.uk \
    --cc=m.szyprowski@samsung.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).