Re: [PATCH v15 00/15] PCI: EP: Add RC-to-EP doorbell with platform MSI controller
On Sat, Mar 01, 2025 at 07:09:35PM +0100, Thomas Gleixner wrote: > On Sat, Mar 01 2025 at 12:02, Marc Zyngier wrote: > > - This IMMUTABLE thing serves no purpose, because you don't randomly > > plug this end-point block on any MSI controller. They come as part > > of an SoC. > > Yes and no. The problem is that the EP implementation is meant to be a > generic library and while GIC-ITS guarantees immutability of the > address/data pair after setup, there are architectures (x86, loongson, > riscv) where the base MSI controller does not and immutability is only > achieved when interrupt remapping is enabled. The latter can be disabled > at boot-time and then the EP implementation becomes a lottery across > affinity changes. > > That was my concern about this library implementation and that's why I > asked for a mechanism to ensure that the underlying irqdomain provides a > immutable address/data pair. > > So it does not matter for GIC-ITS, but in the larger picture it matters. Marc: Do you satisfy Thomans's anwser? So I can respin this series. Frank > > Thanks, > > tglx
Re: [PATCH v15 00/15] PCI: EP: Add RC-to-EP doorbell with platform MSI controller
On Sat, Mar 01 2025 at 12:02, Marc Zyngier wrote: > - This IMMUTABLE thing serves no purpose, because you don't randomly > plug this end-point block on any MSI controller. They come as part > of an SoC. Yes and no. The problem is that the EP implementation is meant to be a generic library and while GIC-ITS guarantees immutability of the address/data pair after setup, there are architectures (x86, loongson, riscv) where the base MSI controller does not and immutability is only achieved when interrupt remapping is enabled. The latter can be disabled at boot-time and then the EP implementation becomes a lottery across affinity changes. That was my concern about this library implementation and that's why I asked for a mechanism to ensure that the underlying irqdomain provides a immutable address/data pair. So it does not matter for GIC-ITS, but in the larger picture it matters. Thanks, tglx
Re: [PATCH v15 00/15] PCI: EP: Add RC-to-EP doorbell with platform MSI controller
On Thu, 20 Feb 2025 20:01:10 +, Frank Li wrote: > > On Tue, Feb 11, 2025 at 02:21:53PM -0500, Frank Li wrote: > > Thomas Gleixner and Marc Zyngier: > > Do you have any comments about irq/msi part? It certainly looks better and less invasive than the previous incarnations. Things to fix: - Documentation: the msi-map property usage is undefined outside of a PCIe RC, and the way you describe its use is so vague I read anything in it. Please update Documentation/devicetree/bindings/pci/pci-msi.txt to reflect the new use case. - This IMMUTABLE thing serves no purpose, because you don't randomly plug this end-point block on any MSI controller. They come as part of an SoC. Thanks, M. -- Without deviation from the norm, progress is not possible.
Re: [PATCH v15 00/15] PCI: EP: Add RC-to-EP doorbell with platform MSI controller
On Thu, Feb 20, 2025 at 09:42:57PM +0100, Thomas Gleixner wrote: > On Thu, Feb 20 2025 at 15:01, Frank Li wrote: > > On Tue, Feb 11, 2025 at 02:21:53PM -0500, Frank Li wrote: > > > > Thomas Gleixner and Marc Zyngier: > > > > Do you have any comments about irq/msi part? > > I'm not having objections, but this needs to be acked by Marc as he had > pretty strong opinions on the overall approach. Marc Zyngier: Do you have any concern about irq/msi part? Is it clean enough to answer what your conern at previous version? How can we move forward? This help PCI EP side avoid software polling status. Frank > > Thanks, > > tglx
Re: [PATCH v15 00/15] PCI: EP: Add RC-to-EP doorbell with platform MSI controller
On Thu, Feb 20 2025 at 15:01, Frank Li wrote: > On Tue, Feb 11, 2025 at 02:21:53PM -0500, Frank Li wrote: > > Thomas Gleixner and Marc Zyngier: > > Do you have any comments about irq/msi part? I'm not having objections, but this needs to be acked by Marc as he had pretty strong opinions on the overall approach. Thanks, tglx
Re: [PATCH v15 00/15] PCI: EP: Add RC-to-EP doorbell with platform MSI controller
On Tue, Feb 11, 2025 at 02:21:53PM -0500, Frank Li wrote: Thomas Gleixner and Marc Zyngier: Do you have any comments about irq/msi part? Frank > ┌┐ ┌───┐ ┌┐ > ││ │ │ ││ > ││ │ PCI Endpoint │ │ PCI Host │ > ││ │ │ ││ > ││◄──┤ 1.platform_msi_domain_alloc_irqs()│ ││ > ││ │ │ ││ > │ MSI├──►│ 2.write_msi_msg() ├──►├─BAR │ > │ Controller │ │ update doorbell register address│ ││ > ││ │ for BAR │ ││ > ││ │ │ │ 3. Write BAR│ > ││◄──┼───┼───┤│ > ││ │ │ ││ > │├──►│ 4.Irq Handle │ ││ > ││ │ │ ││ > ││ │ │ ││ > └┘ └───┘ └┘ > > This patches based on old > https://lore.kernel.org/imx/[email protected]/ > > Original patch only target to vntb driver. But actually it is common > method. > > This patches add new API to pci-epf-core, so any EP driver can use it. > > Previous v2 discussion here. > https://lore.kernel.org/imx/[email protected]/ > > Changes in v15: > - rebase to v6.14-rc1 > - fix build issue find by kernel test robot > - Link to v14: > https://lore.kernel.org/r/[email protected] > > Changes in v14: > Marc Zyngier raised concerns about adding DOMAIN_BUS_DEVICE_PCI_EP_MSI. As > a result, the approach has been reverted to the v9 method. However, there > are several improvements: > > MSI now supports msi-map in addition to msi-parent. > - The struct device: id is used as the endpoint function (EPF) device > identity to map to the stream ID (sideband information). > - The EPC device tree source (DTS) utilizes msi-map to provide such > information. > - The EPF device's of_node is set to the EPC controller’s node. This > approach is commonly used for multi-function device (MFD) platform child > devices, allowing them to inherit properties from the MFD device’s DTS, > such as reset-cells and gpio-cells. This method is well-suited for the > current case, as the EPF is inherently created/binded to the EPC and > should inherit the EPC’s DTS node properties. > > Additionally: > > Since the basic IMX95 LUT support has already been merged into the > mainline, a DTS and driver increment patch is added to complete the > solution. The patch is rebased onto the latest linux-next tree and > aligned with the new pcitest framework. > > - Link to v13: > https://lore.kernel.org/r/[email protected] > > Changes in v13: > - Change to use DOMAIN_BUS_PCI_DEVICE_EP_MSI > - Change request id as func | vfunc << 3 > - Remove IRQ_DOMAIN_MSI_IMMUTABLE > > Thomas Gleixner: > > I hope capture all your points in review comments. If missed, let me know. > > - Link to v12: > https://lore.kernel.org/r/[email protected] > > Changes in v12: > - Change to use IRQ_DOMAIN_MSI_IMMUTABLE and add help function > irq_domain_msi_is_immuatble(). > - split PCI: endpoint: pci-ep-msi: Add MSI address/data pair mutable check to > 3 patches > - Link to v11: > https://lore.kernel.org/r/[email protected] > > Changes in v11: > - Change to use MSI_FLAG_MSG_IMMUTABLE > - Link to v10: > https://lore.kernel.org/r/[email protected] > > Changes in v10: > > Thomas Gleixner: > There are big change in pci-ep-msi.c. I am sure if go on the > corrent path. The key improvement is remove only 1 function devices's > limitation. > > I use new patch for imutable check, which relative additional > feature compared to base enablement patch. > > - Remove patch Add msi_remove_device_irq_domain() in > platform_device_msi_free_irqs_all() > - Add new patch irqchip/gic-v3-its: Avoid overwriting msi_prepare callback if > provided by msi_domain_info > - Remove only support 1 endpoint function limiation. > - Create one MSI domain for each endpoint function devices. > - Use "msi-map" in pci ep controler node, instead of of msi-parent. first > argument is > (func_no << 8 | vfunc_no) > > - Link to v9: > https://lore.kernel.org/r/[email protected] > > Changes in v9 > - Add patch platform-msi: Add msi_remove_device_irq_domain() in > platform_device_msi_free_irqs_all() > - Remove patch PCI: endpoint: Add pci_ep
