Re: [PATCH v4 05/16] iommu: Move bus setup to IOMMU device registration

From: Thierry Reding <treding@nvidia.com>
To: Robin Murphy <robin.murphy@arm.com>, Dmitry Osipenko <digetx@gmail.com>
Cc: Jon Hunter <jonathanh@nvidia.com>,
	joro@8bytes.org, will@kernel.org, iommu@lists.linux.dev,
	linux-arm-kernel@lists.infradead.org, baolu.lu@linux.intel.com,
	kevin.tian@intel.com, suravee.suthikulpanit@amd.com,
	vasant.hegde@amd.com, mjrosato@linux.ibm.com,
	schnelle@linux.ibm.com, linux-kernel@vger.kernel.org,
	"linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>
Subject: Re: [PATCH v4 05/16] iommu: Move bus setup to IOMMU device registration
Date: Thu, 6 Oct 2022 19:12:09 +0200	[thread overview]
Message-ID: <Yz8Mabz/SL7gG9VA@orome> (raw)
In-Reply-To: <583c4689-844d-2ae2-79fb-4750b21c4304@arm.com>

[-- Attachment #1: Type: text/plain, Size: 2611 bytes --]

On Thu, Oct 06, 2022 at 04:27:39PM +0100, Robin Murphy wrote:
> On 2022-10-06 15:01, Jon Hunter wrote:
> > Hi Robin,
> > 
> > On 15/08/2022 17:20, Robin Murphy wrote:
> > > Move the bus setup to iommu_device_register(). This should allow
> > > bus_iommu_probe() to be correctly replayed for multiple IOMMU instances,
> > > and leaves bus_set_iommu() as a glorified no-op to be cleaned up next.
> > > 
> > > At this point we can also handle cleanup better than just rolling back
> > > the most-recently-touched bus upon failure - which may release devices
> > > owned by other already-registered instances, and still leave devices on
> > > other buses with dangling pointers to the failed instance. Now it's easy
> > > to clean up the exact footprint of a given instance, no more, no less.
> > 
> > 
> > Since this change, I have noticed that the DRM driver on Tegra20 is
> > failing to probe and I am seeing ...
> > 
> >   tegra-gr2d 54140000.gr2d: failed to attach to domain: -19
> >   drm drm: failed to initialize 54140000.gr2d: -19
> > 
> > Bisect points to this change and reverting it fixes it. Let me know if
> > you have any thoughts.
> 
> Oh, apparently what's happened is that I've inadvertently enabled the
> tegra-gart driver, since it seems that *wasn't* calling bus_set_iommu()
> before. Looking at the history, it appears to have been that way since
> c7e3ca515e78 ("iommu/tegra: gart: Do not register with bus"), so essentially
> that driver has been broken and useless for close to 8 years now :(
> 
> Given that, I'd be inclined to "fix" it as below, or just give up and delete
> the whole thing.

I'm inclined to agree. GART is severely limited: it provides a single
IOMMU domain with an aperture of 32 MiB. It's close to useless for
anything we would want to do and my understanding is that people have
been falling back to CMA for any graphics/display stuff that the GART
would've been useful for.

Given that nobody's felt the urge to fix this for the past 8 years, I
don't think there's enough interest in this to keep it going.

Dmitry, any thoughts?

Thierry

> 
> Thanks,
> Robin.
> 
> ----->8-----
> diff --git a/drivers/iommu/Kconfig b/drivers/iommu/Kconfig
> index 5c5cb5bee8b6..7b3f7fd6e527 100644
> --- a/drivers/iommu/Kconfig
> +++ b/drivers/iommu/Kconfig
> @@ -230,6 +230,7 @@ config TEGRA_IOMMU_GART
>  	bool "Tegra GART IOMMU Support"
>  	depends on ARCH_TEGRA_2x_SOC
>  	depends on TEGRA_MC
> +	depends on BROKEN
>  	select IOMMU_API
>  	help
>  	  Enables support for remapping discontiguous physical memory

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