linux-kernel - Re: [PATCH v8 2/2] PCI: imx6: Add IOMMU and ITS MSI support for i.MX95

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] [day] [month] [year] [list]

Message-ID: <20250113225905.GA3325507-robh@kernel.org>
Date: Mon, 13 Jan 2025 16:59:05 -0600
From: Rob Herring <robh@...nel.org>
To: Frank Li <Frank.li@....com>
Cc: Lorenzo Pieralisi <lpieralisi@...nel.org>, robin.murphy@....com,
	Bjorn Helgaas <bhelgaas@...gle.com>,
	Richard Zhu <hongxing.zhu@....com>,
	Lucas Stach <l.stach@...gutronix.de>,
	Krzysztof Wilczyński <kw@...ux.com>,
	Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
	Shawn Guo <shawnguo@...nel.org>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Pengutronix Kernel Team <kernel@...gutronix.de>,
	Fabio Estevam <festevam@...il.com>, linux-pci@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	imx@...ts.linux.dev, alyssa@...enzweig.io, bpf@...r.kernel.org,
	broonie@...nel.org, jgg@...pe.ca, joro@...tes.org,
	lgirdwood@...il.com, maz@...nel.org, p.zabel@...gutronix.de,
	will@...nel.org
Subject: Re: [PATCH v8 2/2] PCI: imx6: Add IOMMU and ITS MSI support for
 i.MX95

On Tue, Jan 07, 2025 at 12:03:39PM -0500, Frank Li wrote:
> On Fri, Dec 27, 2024 at 10:58:15AM +0100, Lorenzo Pieralisi wrote:
> > On Tue, Dec 17, 2024 at 10:50:22AM -0500, Frank Li wrote:
> >
> > [...]
> >
> > > > > > Right. Question: what happens if DT shows that there are SMMU and/or
> > > > > > ITS bindings/mappings but the SMMU driver and ITS driver are either not
> > > > > > enabled or have not probed ?
> > > > >
> > > > > It is little bit complex.
> > > > > iommu:
> > > > > Case 1:
> > > > > 	iommu{
> > > > > 		status = "disabled"
> > > > > 	};
> > > > >
> > > > > 	PCI driver normal probed. if RID is in range of iommu-map, not
> > > > > any functional impact and harmless.
> > > > > 	If RID is out of range of iommu-map, "false alarm" will return.
> > > > > enable PCI EP device failure, but actually it can work without IOMMU.
> > > >
> > > > What does "false alarm" mean in practice ? PCI device enable fails
> > > > but actually it should not ?
> > >
> > > Yes, you are right. It should work without iommu. but return failure for
> > > this case.
> >
> > Rob, Robin, are you OK with this patch DT bindings usage (and the
> > related dependencies described in Frank's reply) ?
> >
> > I am referring to "iommu-map" and "msi-map" usage, everything else
> > is platform specific code.
> >
> > It looks like things can break in multiple ways but I don't want
> > to hold up this series forever.
> 
> Rob and Robin:
> 
> 	Let me simple summary situation. PCIe controler driver need config
> "stream id" for each PCI endpoint devices for IOMMU and MSI.
> 
> 	So add callback for host bridge enable/disable an endpoint devices.
> In callback function, call of_map_id("iommu-map" | "msi-map") to get
> devices's "stream id" from pci's rid.  Then config hardware.
> 
> 	The limiation is, if smmu/its controller's "status" is disabled
> and rid is out of the range of "iommu-map" and "msi-map". Enable device
> will be fail although it should be success because "stream id" will not be
> used at all at this case. The out of range of "iommu-map" and "msi-map" is
> rare.
> 
> 	dwc common pci driver simple check "msi-map", which should be
> another limition and not related this patch.
> 
> 	In many dwc platform (like qcom) need config "stream id" also. But
> that direct parse "iommu-map" and "msi-map" by their drivers, which is not
> prefered by Rob now when I try to upstream at v3
> https://lore.kernel.org/imx/20240429150842.GC1709920-robh@kernel.org/
> 
> 	Rob: can you help check if this is correct direction?

My objection was only parsing the property yourself rather than using 
existing functions. So it looks fine to me now. Though you might 
consider if there is something to be shared with QCom driver.

Rob