On Thu, Oct 03, 2024 at 10:45:28AM +0530, Manivannan Sadhasivam wrote: > On Mon, Sep 30, 2024 at 03:42:20PM -0400, Frank Li wrote: > > Some system's IOMMU stream(master) ID bits(such as 6bits) less than > > pci_device_id (16bit). It needs add hardware configuration to enable > > pci_device_id to stream ID convert. > > > > https://lore.kernel.org/imx/20240622173849.GA1432357@bhelgaas/ > > This ways use pcie bus notifier (like apple pci controller), when new PCIe > > device added, bus notifier will call register specific callback to handle > > look up table (LUT) configuration. > > > > https://lore.kernel.org/imx/20240429150842.GC1709920-robh@xxxxxxxxxx/ > > which parse dt's 'msi-map' and 'iommu-map' property to static config LUT > > table (qcom use this way). This way is rejected by DT maintainer Rob. > > > > What is the issue in doing this during the probe() stage? It looks like you are > working with the static info in the devicetree, which is already available > during the controller probe(). There are problems. One: It is not good to manually parser this property in pci host bridge drivers. Two: of_map default is bypass map. For example: if in dts only 2 sid, 0xA and 0xB. If try to enable 3rd function RID(103), there are no error report. of_map will return RID 103 as stream ID. DMA will write to wrong possition possibly. https://elixir.bootlin.com/linux/v6.12-rc1/source/drivers/of/base.c#L2070 Three: LUT resource is limited, if DT provide 16 entry, but LUT have only 8 items, if more device enable, not LUT avaible and can't return error. of course, it may fix dts, but It'd better that driver can handle error properly when meet wrong dtb file. > > > Above ways can resolve LUT take or stream id out of usage the problem. If > > there are not enough stream id resource, not error return, EP hardware > > still issue DMA to do transfer, which may transfer to wrong possition. > > > > Add enable(disable)_device() hook for bridge can return error when not > > enough resource, and PCI device can't enabled. > > > > {enable/disable}_device() doesn't convey the fact you are mapping BDF to SID in > the hardware. Maybe something like, {map/unmap}_bdf2sid() or similar would make > sense. It is called in PCI common code do_pci_enable_device(), hook functin name should be similar with caller. {map/unmap}_bdf2sid() is just implementation in dwc. stream id is only ARM platform concept. May other host bridge do difference thing at enable/disable_device(). So I am still perfer use {enable/disable}_device(). Frank > > - Mani > > > Basicallly this version can match Bjorn's requirement: > > 1: simple, because it is rare that there are no LUT resource. > > 2: EP driver probe failure when no LUT, but lspci can see such device. > > > > [ 2.164415] nvme nvme0: pci function 0000:01:00.0 > > [ 2.169142] pci 0000:00:00.0: Error enabling bridge (-1), continuing > > [ 2.175654] nvme 0000:01:00.0: probe with driver nvme failed with error -12 > > > > > lspci > > 0000:00:00.0 PCI bridge: Philips Semiconductors Device 0000 > > 0000:01:00.0 Non-Volatile memory controller: Micron Technology Inc 2100AI NVMe SSD [Nitro] (rev 03) > > > > To: Bjorn Helgaas <bhelgaas@xxxxxxxxxx> > > To: Richard Zhu <hongxing.zhu@xxxxxxx> > > To: Lucas Stach <l.stach@xxxxxxxxxxxxxx> > > To: Lorenzo Pieralisi <lpieralisi@xxxxxxxxxx> > > To: Krzysztof Wilczyński <kw@xxxxxxxxx> > > To: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxx> > > To: Rob Herring <robh@xxxxxxxxxx> > > To: Shawn Guo <shawnguo@xxxxxxxxxx> > > To: Sascha Hauer <s.hauer@xxxxxxxxxxxxxx> > > To: Pengutronix Kernel Team <kernel@xxxxxxxxxxxxxx> > > To: Fabio Estevam <festevam@xxxxxxxxx> > > Cc: linux-pci@xxxxxxxxxxxxxxx > > Cc: linux-kernel@xxxxxxxxxxxxxxx > > Cc: linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > > Cc: imx@xxxxxxxxxxxxxxx > > Cc: Frank.li@xxxxxxx \ > > Cc: alyssa@xxxxxxxxxxxxx \ > > Cc: bpf@xxxxxxxxxxxxxxx \ > > Cc: broonie@xxxxxxxxxx \ > > Cc: jgg@xxxxxxxx \ > > Cc: joro@xxxxxxxxxx \ > > Cc: l.stach@xxxxxxxxxxxxxx \ > > Cc: lgirdwood@xxxxxxxxx \ > > Cc: maz@xxxxxxxxxx \ > > Cc: p.zabel@xxxxxxxxxxxxxx \ > > Cc: robin.murphy@xxxxxxx \ > > Cc: will@xxxxxxxxxx \ > > > > Signed-off-by: Frank Li <Frank.Li@xxxxxxx> > > --- > > Changes in v2: > > - see each patch > > - Link to v1: https://lore.kernel.org/r/20240926-imx95_lut-v1-0-d0c62087dbab@xxxxxxx > > > > --- > > Frank Li (2): > > PCI: Add enable_device() and disable_device() callbacks for bridges > > PCI: imx6: Add IOMMU and ITS MSI support for i.MX95 > > > > drivers/pci/controller/dwc/pci-imx6.c | 133 +++++++++++++++++++++++++++++++++- > > drivers/pci/pci.c | 14 ++++ > > include/linux/pci.h | 2 + > > 3 files changed, 148 insertions(+), 1 deletion(-) > > --- > > base-commit: 2849622e7b01d5aea1b060ba3955054798c1e0bb > > change-id: 20240926-imx95_lut-1c68222e0944 > > > > Best regards, > > --- > > Frank Li <Frank.Li@xxxxxxx> > > > > > > -- > மணிவண்ணன் சதாசிவம்
