linux-kernel - Re: [PATCH v5 08/12] PCI: imx6: Config look up table(LUT) to support MSI ITS and IOMMU 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] [thread-next>] [day] [month] [year] [list]

Date: Mon, 3 Jun 2024 14:21:27 -0400
From: Frank Li <Frank.li@....com>
To: Bjorn Helgaas <helgaas@...nel.org>
Cc: 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>,
	Marc Zyngier <maz@...nel.org>
Subject: Re: [PATCH v5 08/12] PCI: imx6: Config look up table(LUT) to support
 MSI ITS and IOMMU for i.MX95

On Mon, Jun 03, 2024 at 12:11:49PM -0500, Bjorn Helgaas wrote:
> On Fri, May 31, 2024 at 12:14:18PM -0400, Frank Li wrote:
> > On Thu, May 30, 2024 at 06:08:32PM -0500, Bjorn Helgaas wrote:
> > > 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.
> > 
> > Actually, I refer apple_pcie_bus_notifier(). I can't direct use apple's
> > implement because in imx95 have difference PCI host controller, another one
> > is PCI ECAM netc controller. At lease function name should be similar with
> > apple. 
> 
> I know it's different hardware, so obviously it can't be exactly the
> same.  These are the differences that looked possibly unnecessary:
> 
>   - registering from initcall instead of .probe():
> 
>       apple_pcie_probe                  # .probe() method
>         bus_register_notifier(&pci_bus_type, &apple_pcie_nb)
> 
>       imx_pcie_init                     # device_initcall()
>         bus_register_notifier(&pci_bus_type, &imx_pcie_nb)

Maybe apple only one PCIe controller with multi ports. One nb just register
once.

If there are multi PCIe controller instance, second bus_register_notifier()
report error.

Of course, if you like, I can use global reference counter to handle this.

> 
>   - naming BUS_NOTIFY_DEL_DEVICE function:
> 
>       apple_pcie_release_device()
>       imx_pcie_del_device()

Okay, I can change to release_device().

> 
>   - tracking entries in use via bitmap vs scanning hardware for
>     invalid entries:
> 
>       bitmap_find_free_region           # apple
> 
>       imx_pcie_config_lut               # imx
>         for (i = 0; i < IMX95_MAX_LUT; i++)
>           regmap_read(imx_pcie->iomuxc_gpr, IMX95_PE0_LUT_DATA1, &data1)
>           if (data1 & IMX95_PE0_LUT_VLD)
>             continue

Okay, I can change to use bitmap.

> 
> When we fix a bug in one driver, it's easier to check whether other
> drivers also need the fix if they use the same structure and names.
> 
> Bjorn