> -----Original Message----- > From: Thomas Gleixner <tglx@xxxxxxxxxxxxx> > Sent: Monday, November 28, 2022 1:15 PM > To: Frank Li <frank.li@xxxxxxx>; lpieralisi@xxxxxxxxxx > Cc: Frank Li <frank.li@xxxxxxx>; Aisheng Dong <aisheng.dong@xxxxxxx>; > bhelgaas@xxxxxxxxxx; devicetree@xxxxxxxxxxxxxxx; festevam@xxxxxxxxx; > imx@xxxxxxxxxxxxxxx; jdmason@xxxxxxxx; kernel@xxxxxxxxxxxxxx; > kishon@xxxxxx; krzysztof.kozlowski+dt@xxxxxxxxxx; kw@xxxxxxxxx; linux- > arm-kernel@xxxxxxxxxxxxxxxxxxx; dl-linux-imx <linux-imx@xxxxxxx>; linux- > kernel@xxxxxxxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; > lorenzo.pieralisi@xxxxxxx; lznuaa@xxxxxxxxx; > manivannan.sadhasivam@xxxxxxxxxx; maz@xxxxxxxxxx; ntb@xxxxxxxxxxxxxxx; > Peng Fan <peng.fan@xxxxxxx>; robh+dt@xxxxxxxxxx; > s.hauer@xxxxxxxxxxxxxx; shawnguo@xxxxxxxxxx > Subject: [EXT] Re: [PATCH v13 2/2] PCI: endpoint: pci-epf-vntb: using platform > MSI as doorbell > > Caution: EXT Email > > On Thu, Nov 24 2022 at 00:50, Frank Li wrote: > > ┌────────────┐ ┌─────────────── > ────────────────────┐ ┌───────── > ───────┐ > > │ │ │ │ │ │ > > │ │ │ PCI Endpoint │ │ PCI Host │ > > │ │ │ │ │ │ > > │ │◄──┤ 1.platform_msi_domain_alloc_irqs()│ │ > │ > > │ │ │ │ │ │ > > │ MSI ├──►│ 2.write_msi_msg() ├──►├─BAR<n> > │ > > │ Controller │ │ update doorbell register address│ │ │ > > │ │ │ for BAR │ │ │ > > │ │ │ │ │ 3. Write BAR<n>│ > > │ │◄──┼────────────────────── > ─────────────┼───┤ │ > > │ │ │ │ │ │ > > │ ├──►│ 4.Irq Handle │ │ │ > > │ │ │ │ │ │ > > │ │ │ │ │ │ > > └────────────┘ └─────────────── > ────────────────────┘ └───────── > ───────┘ > > > > Using platform MSI interrupt controller as endpoint(EP)'s doorbell. > > Can you please explain what the MSI controller is in this picture? MSI > controller is not a term which is common in the interrupt handling > landscape despite the fact that it's pretty wide spread in device tree > bindings presumably through intensive copy & pasta cargo cult. I use irq-imx-mu-msi to do test. I supposed it should work for all kinds general msi controller. DTS part still not upstream yet because PCI EP enable patches still be under review. https://lore.kernel.org/all/1663913220-9523-1-git-send-email-hongxing.zhu@xxxxxxx/ Our test platform have not GIC ITS supported yet. > > > Basic working follow as > > 1. EP function driver call platform_msi_domain_alloc_irqs() alloc a > > MSI irq from MSI controller with call back function write_msi_msg(); > > 2. write_msg_msg will config BAR and map to address defined in msi_msg; > > 3. Host side trigger an IRQ at Endpoint by write to BAR region. > > You're explaining what the code does, but fail to explain the underlying > mechanisms. > > Platform MSI is definitely the wrong mechanism here. Why? This patch use Platform MSI. I never said " Platform MSI is definitely the wrong mechanism here". Base logic is that, get msi controller's message address by irq API. Map this physical address to DB BAR, so PCI host write this DB bar, then EP generate irq. You can refer v14 version, which should be better description. > > This is about a PCIe endpoint, which is usually handled by a PCI/MSI > interrupt domain. Obviously this usage does not fit into the way how the > "global" PCI/MSI domains work. PCI endpoint have not standard PCI configure space to enable/disable MSI irq and MSI address (CAP 05). That's reason why need "platform msi", or you called "global" Now. > > There is upcoming work and at least the generic parts should show up in > 6.2 which addresses exactly the problem you are trying to solve: > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.k > ernel.org%2Fall%2F20221124225331.464480443%40linutronix.de&data > =05%7C01%7CFrank.Li%40nxp.com%7C6a07e33e56af45ffc1ff08dad174d02d > %7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6380525969049530 > 06%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMz > IiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=Q8jr > eVGGLa2M4yhjGO7Njqwdm59XDC0GyLEwkr0k6B0%3D&reserved=0 > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.k > ernel.org%2Fall%2F20221124230505.073418677%40linutronix.de&data > =05%7C01%7CFrank.Li%40nxp.com%7C6a07e33e56af45ffc1ff08dad174d02d > %7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6380525969049530 > 06%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMz > IiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=Tc9p > XNJ499ETFgNWQBNLViFk8D5GbvrrwYDlBW%2Bf2qg%3D&reserved=0 > > plus the prove that the platform MSI mess can be replaced by this: > > > https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.k > ernel.org%2Fall%2F20221121135653.208611233%40linutronix.de&data > =05%7C01%7CFrank.Li%40nxp.com%7C6a07e33e56af45ffc1ff08dad174d02d > %7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C6380525969049530 > 06%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMz > IiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=R5K > NVfcGqxoCam%2FYhY57ihsloWGhGLM3Kh9IkyME4lk%3D&reserved=0 > > NTB in it's current form should never have happened, but that's water > down the bridge. > > What you really want is: > > 1) Convert your platform to the new MSI parent model > > 2) Utilize PCI/IMS which is giving you exactly what you need with > proper PCI semantics Sorry, I still don't understand yet. This patch is just user of msi controller. Your patches focus on the msi controller itself. Interface platform_msi_domain_alloc_irqs still exist at your devmsi-v2-part3. > > Thanks, > > tglx
