linux-kernel - Re: [PATCH 16/18] staging/media/tegra-vde: Clean up IOMMU workaround

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 20 Aug 2020 22:51:02 +0300
From:   Dmitry Osipenko <digetx@...il.com>
To:     Robin Murphy <robin.murphy@....com>, hch@....de, joro@...tes.org,
        linux@...linux.org.uk
Cc:     will@...nel.org, inki.dae@...sung.com, sw0312.kim@...sung.com,
        kyungmin.park@...sung.com, m.szyprowski@...sung.com,
        agross@...nel.org, bjorn.andersson@...aro.org,
        thierry.reding@...il.com, jonathanh@...dia.com, vdumpa@...dia.com,
        matthias.bgg@...il.com, yong.wu@...iatek.com,
        geert+renesas@...der.be, magnus.damm@...il.com, t-kristo@...com,
        s-anna@...com, laurent.pinchart@...asonboard.com,
        linux-arm-kernel@...ts.infradead.org,
        iommu@...ts.linux-foundation.org,
        linux-samsung-soc@...r.kernel.org, linux-tegra@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, linux-mediatek@...ts.infradead.org,
        dri-devel@...ts.freedesktop.org, linux-media@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH 16/18] staging/media/tegra-vde: Clean up IOMMU workaround

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@....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. 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.

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.

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.