Date: Fri, 31 May 2024 14:52:50 +0100
From: Marc Zyngier <maz@...nel.org>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: Frank Li <Frank.Li@....com>,	Richard Zhu <hongxing.zhu@....com>,	Lucas
 Stach <l.stach@...gutronix.de>,	Lorenzo Pieralisi <lpieralisi@...nel.org>,
	Krzysztof Wilczyński <kw@...ux.com>,	Rob Herring
 <robh@...nel.org>,	Bjorn Helgaas <bhelgaas@...gle.com>,	Shawn Guo
 <shawnguo@...nel.org>,	Sascha Hauer <s.hauer@...gutronix.de>,	Pengutronix
 Kernel Team <kernel@...gutronix.de>,	Fabio Estevam <festevam@...il.com>,
	NXP Linux Team <linux-imx@....com>,	Philipp Zabel <p.zabel@...gutronix.de>,
	Liam Girdwood <lgirdwood@...il.com>,	Mark Brown <broonie@...nel.org>,
	Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,	Krzysztof
 Kozlowski <krzysztof.kozlowski+dt@...aro.org>,	Conor Dooley
 <conor+dt@...nel.org>,	linux-pci@...r.kernel.org,	imx@...ts.linux.dev,
	linux-arm-kernel@...ts.infradead.org,	linux-kernel@...r.kernel.org,
	bpf@...r.kernel.org,	devicetree@...r.kernel.org,	Will Deacon
 <will@...nel.org>,	Robin Murphy <robin.murphy@....com>,	Joerg Roedel
 <joro@...tes.org>,	Jason Gunthorpe <jgg@...pe.ca>,	Alyssa Rosenzweig
 <alyssa@...enzweig.io>
Subject: Re: [PATCH v5 08/12] PCI: imx6: Config look up table(LUT) to support MSI ITS and IOMMU for i.MX95

On Fri, 31 May 2024 00:08:32 +0100,
Bjorn Helgaas <helgaas@...nel.org> wrote:
> 
> [+cc IOMMU and pcie-apple.c folks for comment]
> 
> On Tue, May 28, 2024 at 03:39:21PM -0400, Frank Li wrote:
> > For the i.MX95, configuration of a LUT is necessary to convert Bus Device
> > Function (BDF) to stream IDs, which are utilized by both IOMMU and ITS.
> > This involves examining the msi-map and smmu-map to ensure consistent
> > mapping of PCI BDF to the same stream IDs. Subsequently, LUT-related
> > registers are configured. In the absence of an msi-map, the built-in MSI
> > controller is utilized as a fallback.
> > 
> > Additionally, register a PCI bus notifier to trigger imx_pcie_add_device()
> > upon the appearance of a new PCI device and when the bus is an iMX6 PCI
> > controller. This function configures the correct LUT based on Device Tree
> > Settings (DTS).
> 
> This scheme is pretty similar to apple_pcie_bus_notifier().  If we
> have to do this, I wish it were *more* similar, i.e., copy the
> function names, bitmap tracking, code structure, etc.
> 
> I don't really know how stream IDs work, but I assume they are used on
> most or all arm64 platforms, so I'm a little surprised that of all the
> PCI host drivers used on arm64, only pcie-apple.c and pci-imx6.c need
> this notifier.

That's because on sane platforms, PCI's RID and the IOMMU's SID are
the same thing, and there is no need to do anything bizarre. In the
worse case, there is a static transformation that can be applied (IORT
for ACPI, or iommu-map for DT).

Some "creative" systems such as the Apple stuff require some extra
side-band configuration because they don't know what a RID is at all
(the RID isn't conveyed to the IOMMU), nor is there a static (baked in
HW) transformation between RID and SID.

So there is a widget on the side that performs the conversion, and
this widget needs to be programmed. The way it works is that the
driver parses the iommu-map to find the expected and arbitrary) SID
on the IOMMU side for a given RID, and programs the association in the
RID-to-SID mapper. It does so at device probe time in order to make
sure the widget is alive (it seems to be part of the port power
domain).

Yes, this is a terrible hack, and I wish we didn't have to deal with
this sort of crap.

>
> There's this path, which is pretty generic and does at least the
> of_map_id() part of what you're doing in imx_pcie_add_device():
> 
>     __driver_probe_device
>       really_probe
>         pci_dma_configure                       # pci_bus_type.dma_configure
>           of_dma_configure
>             of_dma_configure_id
>               of_iommu_configure
>                 of_pci_iommu_init
>                   of_iommu_configure_dev_id
>                     of_map_id
>                     of_iommu_xlate
>                       ops = iommu_ops_from_fwnode
>                       iommu_fwspec_init
>                       ops->of_xlate(dev, iommu_spec)
> 
> Maybe this needs to be extended somehow with a hook to do the
> device-specific work like updating the LUT?  Just speculating here,
> the IOMMU folks will know how this is expected to work.

That'd be a possibility. But this would be adding extra complexity to
the IOMMU core, and I'm not sure it is worth it given that these
systems are thankfully "rare".

It is also something that is conceptually part of the PCIe root port,
and not really the IOMMU, so I'm a bit reluctant to shove things
there. In any case, it would still require callbacks into the PCIe
host driver, and I have the feeling that we'd be reinventing the bus
notifier wheel, only with pointier angles...

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

Powered by blists - more mailing lists