On Mon, Jun 03, 2024 at 11:29:38PM +0300, Laurentiu Tudor wrote: > > > On 5/31/24 17:58, Robin Murphy wrote: > > On 2024-05-31 12:08 am, Bjorn Helgaas 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. > > > > This is one of those things that's mostly at the mercy of the PCIe root > > complex implementation. Typically the SMMU StreamID and/or GIC ITS > > DeviceID is derived directly from the PCI RID, sometimes with additional > > high-order bits hard-wired to disambiguate PCI segments. I believe this > > RID-translation LUT is a particular feature of the the Synopsys IP - I > > know there's also one on the NXP Layerscape platforms, but on those it's > > programmed by the bootloader, which also generates the appropriate > > "msi-map" and "iommu-map" properties to match. Ideally that's what i.MX > > should do as well, but hey. > > That's usually fine, except when SRIOV and/or hotplug devices (that is, not > discoverable at bootloader time) come into play. We came up with this > "solution" to cover these more dynamic scenarios. > > https://source.denx.de/u-boot/u-boot/-/commit/2a5bbb13cc39102a68fcc31056925427ab44b591 If my understand is correct, basic it was still pre-allocate method. Just reserve more streams id for SRIOV and hotplugs. It was not really depend on what devices in system. Frank > > --- > Best Regards, Laurentiu > > > > 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. > > > > Note that that particular code path has fundamental issues and much of > > it needs to go away (I'm working on it, but it's a rich ~8-year-old pile > > of technical debt...). IOMMU configuration needs to be happening at > > device_add() time via the IOMMU layer's own bus notifier. > > > > If it's really necessary to do this programming from Linux, then there's > > still no point in it being dynamic - the mappings cannot ever change, > > since the rest of the kernel believes that what the DT said at boot time > > was already a property of the hardware. It would be a lot more logical, > > and likely simpler, for the driver to just read the relevant map > > property and program the entire LUT to match, all in one go at > > controller probe time. Rather like what's already commonly done with the > > parsing of "dma-ranges" to program address-translation LUTs for inbound > > windows. > > > > Plus that would also give a chance of safely dealing with bad DTs > > specifying invalid ID mappings (by refusing to probe at all). As it is, > > returning an error from a child's BUS_NOTIFY_ADD_DEVICE does nothing > > except prevent any further notifiers from running at that point - the > > device will still be added, allowed to bind a driver, and able to start > > sending DMA/MSI traffic without the controller being correctly > > programmed, which at best won't work and at worst may break the whole > > system. > > > > Thanks, > > Robin. > > > > _______________________________________________ > > linux-arm-kernel mailing list > > linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel