IOMMU Archive on lore.kernel.org
 help / color / Atom feed
From: Robin Murphy <robin.murphy@arm.com>
To: Dmitry Osipenko <digetx@gmail.com>,
	hch@lst.de, joro@8bytes.org, linux@armlinux.org.uk
Cc: geert+renesas@glider.be, dri-devel@lists.freedesktop.org,
	matthias.bgg@gmail.com, thierry.reding@gmail.com,
	laurent.pinchart@ideasonboard.com, will@kernel.org,
	linux-samsung-soc@vger.kernel.org, magnus.damm@gmail.com,
	kyungmin.park@samsung.com, jonathanh@nvidia.com,
	agross@kernel.org, linux-media@vger.kernel.org,
	linux-arm-msm@vger.kernel.org, inki.dae@samsung.com,
	linux-mediatek@lists.infradead.org, linux-tegra@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, sw0312.kim@samsung.com,
	linux-kernel@vger.kernel.org, t-kristo@ti.com,
	iommu@lists.linux-foundation.org
Subject: Re: [PATCH 16/18] staging/media/tegra-vde: Clean up IOMMU workaround
Date: Fri, 21 Aug 2020 01:11:57 +0100
Message-ID: <cb12808b-7316-19db-7413-b7f852a6f8ae@arm.com> (raw)
In-Reply-To: <07135a55-cbc9-83e5-60dc-731282192554@gmail.com>

On 2020-08-20 20:51, Dmitry Osipenko wrote:
> 20.08.2020 18:08, Robin Murphy пишет:
>> Now that arch/arm is wired up for default domains and iommu-dma, we no
>> longer need to work around the arch-private mapping.
>>
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>> ---
>>   drivers/staging/media/tegra-vde/iommu.c | 12 ------------
>>   1 file changed, 12 deletions(-)
>>
>> diff --git a/drivers/staging/media/tegra-vde/iommu.c b/drivers/staging/media/tegra-vde/iommu.c
>> index 6af863d92123..4f770189ed34 100644
>> --- a/drivers/staging/media/tegra-vde/iommu.c
>> +++ b/drivers/staging/media/tegra-vde/iommu.c
>> @@ -10,10 +10,6 @@
>>   #include <linux/kernel.h>
>>   #include <linux/platform_device.h>
>>   
>> -#if IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)
>> -#include <asm/dma-iommu.h>
>> -#endif
>> -
>>   #include "vde.h"
>>   
>>   int tegra_vde_iommu_map(struct tegra_vde *vde,
>> @@ -70,14 +66,6 @@ int tegra_vde_iommu_init(struct tegra_vde *vde)
>>   	if (!vde->group)
>>   		return 0;
>>   
>> -#if IS_ENABLED(CONFIG_ARM_DMA_USE_IOMMU)
>> -	if (dev->archdata.mapping) {
>> -		struct dma_iommu_mapping *mapping = to_dma_iommu_mapping(dev);
>> -
>> -		arm_iommu_detach_device(dev);
>> -		arm_iommu_release_mapping(mapping);
>> -	}
>> -#endif
>>   	vde->domain = iommu_domain_alloc(&platform_bus_type);
>>   	if (!vde->domain) {
>>   		err = -ENOMEM;
>>
> 
> Hello, Robin! Thank you for yours work!
> 
> Some drivers, like this Tegra VDE (Video Decoder Engine) driver for
> example, do not want to use implicit IOMMU domain.

That isn't (intentionally) changing here - the only difference should be 
that instead of having the ARM-special implicit domain, which you have 
to kick out of the way with the ARM-specific API before you're able to 
attach your own domain, the implicit domain is now a proper IOMMU API 
default domain, which automatically gets bumped by your attach. The 
default domains should still only be created in the same cases that the 
ARM dma_iommu_mappings were.

> Tegra VDE driver
> relies on explicit IOMMU domain in a case of Tegra SMMU because VDE
> hardware can't access last page of the AS and because driver wants to
> reserve some fixed addresses [1].
> 
> [1]
> https://elixir.bootlin.com/linux/v5.9-rc1/source/drivers/staging/media/tegra-vde/iommu.c#L100
> 
> Tegra30 SoC supports up to 4 domains, hence it's not possible to afford
> wasting unused implicit domains. I think this needs to be addressed
> before this patch could be applied.

Yeah, there is one subtle change in behaviour from removing the ARM 
layer on top of the core API, in that the IOMMU driver will no longer 
see an explicit detach call. Thus it does stand to benefit from being a 
bit cleverer about noticing devices being moved from one domain to 
another by an attach call, either by releasing the hardware context for 
the inactive domain once the device(s) are moved across to the new one, 
or by simply reprogramming the hardware context in-place for the new 
domain's address space without allocating a new one at all (most of the 
drivers that don't have multiple contexts already handle the latter 
approach quite well).

> Would it be possible for IOMMU drivers to gain support for filtering out
> devices in iommu_domain_alloc(dev, type)? Then perhaps Tegra SMMU driver
> could simply return NULL in a case of type=IOMMU_DOMAIN_DMA and
> dev=tegra-vde.

If you can implement IOMMU_DOMAIN_IDENTITY by allowing the relevant 
devices to bypass translation entirely without needing a hardware 
context (or at worst, can spare one context which all identity-mapped 
logical domains can share), then you could certainly do that kind of 
filtering with the .def_domain_type callback if you really wanted to. As 
above, the intent is that that shouldn't be necessary for this 
particular case, since only one of a group's default domain and 
explicitly attached domain can be live at any given time, so the driver 
should be able to take advantage of that.

If you simply have more active devices (groups) than available contexts 
then yes, you probably would want to do some filtering to decide who 
deserves a translation domain and who doesn't, but in that case you 
should already have had a long-standing problem with the ARM implicit 
domains.

> Alternatively, the Tegra SMMU could be changed such that the devices
> will be attached to a domain at the time of a first IOMMU mapping
> invocation instead of attaching at the time of attach_dev() callback
> invocation.
> 
> Or maybe even IOMMU core could be changed to attach devices at the time
> of the first IOMMU mapping invocation? This could be a universal
> solution for all drivers.

I suppose technically you could do that within an IOMMU driver already 
(similar to how some defer most of setup that logically belongs to 
->domain_alloc until the first ->attach_dev). It's a bit grim from the 
caller's PoV though, in terms of the failure mode being non-obvious and 
having no real way to recover. Again, you'd be better off simply making 
decisions up-front at domain_alloc or attach time based on the domain type.

Robin.
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

  parent reply index

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CGME20200820150857eucas1p18f5f2ad87703a68b6ed20a090f7c1c57@eucas1p1.samsung.com>
2020-08-20 15:08 ` [PATCH 00/18] Convert arch/arm to use iommu-dma Robin Murphy
2020-08-20 15:08   ` [PATCH 01/18] ARM/dma-mapping: Drop .dma_supported for IOMMU ops Robin Murphy
2020-08-20 15:08   ` [PATCH 02/18] ARM/dma-mapping: Consolidate IOMMU ops callbacks Robin Murphy
2020-08-20 15:08   ` [PATCH 03/18] ARM/dma-mapping: Merge IOMMU ops Robin Murphy
2020-08-20 15:08   ` [PATCH 04/18] iommu/dma: Add temporary hacks for arch/arm Robin Murphy
2020-08-20 15:08   ` [PATCH 05/18] ARM/dma-mapping: Switch to iommu_dma_ops Robin Murphy
2020-09-28 11:32     ` Marek Szyprowski
2020-08-20 15:08   ` [PATCH 06/18] ARM/dma-mapping: Support IOMMU default domains Robin Murphy
2020-08-20 15:08   ` [PATCH 07/18] iommu/arm-smmu: Remove arch/arm workaround Robin Murphy
2020-08-21  8:07     ` Will Deacon
2020-08-20 15:08   ` [PATCH 08/18] iommu/renesas: " Robin Murphy
2020-08-20 15:08   ` [PATCH 09/18] iommu/mediatek-v1: Add IOMMU_DOMAIN_DMA support Robin Murphy
2020-08-29  9:54     ` Yong Wu
2020-08-20 15:08   ` [PATCH 10/18] iommu/msm: " Robin Murphy
2020-08-20 15:55     ` Rob Clark
2020-08-20 16:58       ` Robin Murphy
2020-08-20 17:05         ` Rob Clark
2020-08-20 15:08   ` [PATCH 11/18] iommu/omap: " Robin Murphy
2020-08-24 21:39     ` Suman Anna via iommu
2020-08-20 15:08   ` [PATCH 12/18] iommu/tegra-gart: " Robin Murphy
2020-08-20 20:16     ` Dmitry Osipenko
2020-08-21  0:28       ` Robin Murphy
2020-08-23 21:42         ` Dmitry Osipenko
2020-08-20 15:08   ` [PATCH 13/18] iommu/tegra: " Robin Murphy
2020-08-27 15:45     ` Thierry Reding
2020-08-27 18:18       ` Robin Murphy
2020-08-20 15:08   ` [PATCH 14/18] drm/exynos: Consolidate IOMMU mapping code Robin Murphy
2020-09-18 14:30     ` Marek Szyprowski
2020-09-21  2:09     ` Inki Dae
2020-08-20 15:08   ` [PATCH 15/18] drm/nouveau/tegra: Clean up IOMMU workaround Robin Murphy
2020-08-20 15:08   ` [PATCH 16/18] staging/media/tegra-vde: " Robin Murphy
2020-08-20 19:51     ` Dmitry Osipenko
2020-08-20 20:10       ` Dmitry Osipenko
2020-08-21  0:11       ` Robin Murphy [this message]
2020-08-23 21:34         ` Dmitry Osipenko
2020-08-24 14:01           ` Robin Murphy
2020-08-27  7:05             ` Dmitry Osipenko
2020-08-27 15:54               ` Thierry Reding
2020-08-30 19:44                 ` Dmitry Osipenko
2020-08-20 15:08   ` [PATCH 17/18] media/omap3isp: " Robin Murphy
2020-08-20 16:53     ` Sakari Ailus
2020-08-20 17:25       ` Robin Murphy
2020-08-20 19:55         ` Sakari Ailus
2020-08-20 23:01           ` Robin Murphy
2020-08-24 21:55             ` Suman Anna via iommu
2020-08-20 15:08   ` [PATCH 18/18] ARM/dma-mapping: Remove legacy dma-iommu API Robin Murphy
2020-08-24 11:40   ` [PATCH 00/18] Convert arch/arm to use iommu-dma Marek Szyprowski
2020-09-18 15:13     ` Marek Szyprowski
2020-08-27 12:31   ` Aw: " Frank Wunderlich
2020-08-27 12:54     ` Matthias Brugger

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=cb12808b-7316-19db-7413-b7f852a6f8ae@arm.com \
    --to=robin.murphy@arm.com \
    --cc=agross@kernel.org \
    --cc=digetx@gmail.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=geert+renesas@glider.be \
    --cc=hch@lst.de \
    --cc=inki.dae@samsung.com \
    --cc=iommu@lists.linux-foundation.org \
    --cc=jonathanh@nvidia.com \
    --cc=joro@8bytes.org \
    --cc=kyungmin.park@samsung.com \
    --cc=laurent.pinchart@ideasonboard.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-arm-msm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=linux-samsung-soc@vger.kernel.org \
    --cc=linux-tegra@vger.kernel.org \
    --cc=linux@armlinux.org.uk \
    --cc=magnus.damm@gmail.com \
    --cc=matthias.bgg@gmail.com \
    --cc=sw0312.kim@samsung.com \
    --cc=t-kristo@ti.com \
    --cc=thierry.reding@gmail.com \
    --cc=will@kernel.org \
    /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

IOMMU Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-iommu/0 linux-iommu/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-iommu linux-iommu/ https://lore.kernel.org/linux-iommu \
		iommu@lists.linux-foundation.org
	public-inbox-index linux-iommu

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.linux-foundation.lists.iommu


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git