Re: [PATCH 1/5] iommu/tegra-smmu: Fix domain_alloc

[Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

[-- Attachment #1.1: Type: text/plain, Size: 2014 bytes --]

On Wed, Jan 16, 2019 at 12:50:10PM -0800, Navneet Kumar wrote:
> * Allocate dma iova cookie for a domain while adding dma iommu
> devices.
> * Perform a stricter check for domain type parameter.
> 
> Signed-off-by: Navneet Kumar <navneetk-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
> ---
>  drivers/iommu/tegra-smmu.c | 43 +++++++++++++++++++++++++++----------------
>  1 file changed, 27 insertions(+), 16 deletions(-)

I just gave this a quick spin because I was investigating how we could
potentially make use of the DMA API instead of the IOMMU API directly in
Tegra DRM. We currently rely on the fact that the Tegra SMMU driver only
supports unmanaged domains. Once we start supporting DMA domains all the
automatic machinery kicks in and there's lots of SMMU faults. I think at
least partially those faults point out bugs we currently have in the
code. From the looks of it, the display controller is running during
boot and happily fetching from whatever address it was configured with
in the bootloader, and when we enable the ASID for the display
controller as part of the DMA/IOMMU setup, the fetches from the display
controller will be accessing IOV addresses that don't have a mapping.

One one hand that's a good thing because it points out existing
weaknesses, but then it also means that we can't merge this series
because it causes bad regressions.

I also see failures from the GPU with this applied, which I think stem
from the fact that we're now transparently mapping allocations through
the SMMU without the Nouveau driver knowing that and setting the
appropriate bit when addressing memory. Or it could come from the SMMU
code in Nouveau trying to map an already mapped buffer, so effectively
creating an IOVA mapping to an address that is already a IOV address
rather than a physical address.

So I think before we can go ahead with this series we have a lot of
janitorial work to do first so that this won't cause any regressions
when applied.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]