| Message ID | 20241209-ep-msi-v11-3-7434fa8397bd@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | PCI: EP: Add RC-to-EP doorbell with platform MSI controller | expand |
Frank! On Mon, Dec 09 2024 at 12:48, Frank Li wrote: > Some MSI controller change address/data pair when irq_set_affinity(). > Current PCI endpoint can't support this type MSI controller. So add flag > MSI_FLAG_MSG_IMMUTABLE in include/linux/msi.h, set this flags in ARM GIC > ITS MSI controller and check it when allocate doorbell. > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > --- > Change from v9 to v11 > - new patch > --- > drivers/irqchip/irq-gic-v3-its-msi-parent.c | 1 + > drivers/pci/endpoint/pci-ep-msi.c | 8 ++++++++ > include/linux/msi.h | 2 ++ This is not how it works and I explained that you to more than once in the past. The change to the interrupt code is standalone and has nothing to do with PCI endpoint. The latter is just a user of this. So this want's to be split into several patches: 1) add the new flag and a helper function which checks for the flag. 2) add the flag to GICv3 ITS with a proper explanation 3) Check for it in the PCI endpoint code > > + if (!dom->msi_parent_ops) > + return -EINVAL; irq_domain_is_msi_parent() > + if (!(dom->msi_parent_ops->supported_flags & MSI_FLAG_MSG_IMMUTABLE)) { Want's a helper. Thanks, tglx
On Mon, Dec 09, 2024 at 08:51:52PM +0100, Thomas Gleixner wrote: > Frank! > > On Mon, Dec 09 2024 at 12:48, Frank Li wrote: > > > Some MSI controller change address/data pair when irq_set_affinity(). > > Current PCI endpoint can't support this type MSI controller. So add flag > > MSI_FLAG_MSG_IMMUTABLE in include/linux/msi.h, set this flags in ARM GIC > > ITS MSI controller and check it when allocate doorbell. > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > --- > > Change from v9 to v11 > > - new patch > > --- > > drivers/irqchip/irq-gic-v3-its-msi-parent.c | 1 + > > drivers/pci/endpoint/pci-ep-msi.c | 8 ++++++++ > > include/linux/msi.h | 2 ++ > > This is not how it works and I explained that you to more than once in > the past. The below suggest is great and I will update at next version. But I want to know where I lost this. v10: https://lore.kernel.org/imx/20241204-ep-msi-v10-0-87c378dbcd6d@nxp.com/T/#m3060eeab5d5bac931db685d21755a827dfc7a8c7 Talk about MUTABLE or IMPUTABLE v9: https://lore.kernel.org/imx/87v7w0s9a8.ffs@tglx/ Talk about need add a imputable flags v8: https://lore.kernel.org/imx/20241116-ep-msi-v8-0-6f1f68ffd1bb@nxp.com/ v7: https://lore.kernel.org/imx/20241114-ep-msi-v7-0-d4ac7aafbd2c@nxp.com/ v6: https://lore.kernel.org/all/20241112-ep-msi-v6-0-45f9722e3c2a@nxp.com/ v5: https://lore.kernel.org/all/20241108-ep-msi-v5-0-a14951c0d007@nxp.com/ v4: https://lore.kernel.org/all/20241031-ep-msi-v4-0-717da2d99b28@nxp.com/ Have not found your comments at v4 - v8 v3: https://lore.kernel.org/all/20241015-ep-msi-v3-0-cedc89a16c1a@nxp.com/ Talk about msi_get_virq() Frank > > The change to the interrupt code is standalone and has nothing to do > with PCI endpoint. The latter is just a user of this. > > So this want's to be split into several patches: > > 1) add the new flag and a helper function which checks for the flag. > > 2) add the flag to GICv3 ITS with a proper explanation > > 3) Check for it in the PCI endpoint code > > > > > + if (!dom->msi_parent_ops) > > + return -EINVAL; > > irq_domain_is_msi_parent() > > > + if (!(dom->msi_parent_ops->supported_flags & MSI_FLAG_MSG_IMMUTABLE)) { > > Want's a helper. > > Thanks, > > tglx
On Mon, Dec 09, 2024 at 08:51:52PM +0100, Thomas Gleixner wrote: > Frank! > > On Mon, Dec 09 2024 at 12:48, Frank Li wrote: > > > Some MSI controller change address/data pair when irq_set_affinity(). > > Current PCI endpoint can't support this type MSI controller. So add flag > > MSI_FLAG_MSG_IMMUTABLE in include/linux/msi.h, set this flags in ARM GIC > > ITS MSI controller and check it when allocate doorbell. > > > > Signed-off-by: Frank Li <Frank.Li@nxp.com> > > --- > > Change from v9 to v11 > > - new patch > > --- > > drivers/irqchip/irq-gic-v3-its-msi-parent.c | 1 + > > drivers/pci/endpoint/pci-ep-msi.c | 8 ++++++++ > > include/linux/msi.h | 2 ++ > > This is not how it works and I explained that you to more than once in > the past. > > The change to the interrupt code is standalone and has nothing to do > with PCI endpoint. The latter is just a user of this. > > So this want's to be split into several patches: > > 1) add the new flag and a helper function which checks for the flag. msi.h have not included irqdomain.h. It needs a API in msi.c +bool msi_domain_is_immutable(struct irq_domain *domain) +{ + if (!irq_domain_is_msi_parent(domain)) + return false; + + return domain->msi_parent_ops->supported_flags & MSI_FLAG_MSG_IMMUTABLE; +} +EXPORT_SYMBOL_GPL(msi_domain_is_immutable); Is it okay? Frank > > 2) add the flag to GICv3 ITS with a proper explanation > > 3) Check for it in the PCI endpoint code > > > > > + if (!dom->msi_parent_ops) > > + return -EINVAL; > > irq_domain_is_msi_parent() > > > + if (!(dom->msi_parent_ops->supported_flags & MSI_FLAG_MSG_IMMUTABLE)) { > > Want's a helper. > > Thanks, > > tglx
diff --git a/drivers/irqchip/irq-gic-v3-its-msi-parent.c b/drivers/irqchip/irq-gic-v3-its-msi-parent.c index 33e94cfc4d506..a34e9bf93f991 100644 --- a/drivers/irqchip/irq-gic-v3-its-msi-parent.c +++ b/drivers/irqchip/irq-gic-v3-its-msi-parent.c @@ -16,6 +16,7 @@ #define ITS_MSI_FLAGS_SUPPORTED (MSI_GENERIC_FLAGS_MASK | \ MSI_FLAG_PCI_MSIX | \ + MSI_FLAG_MSG_IMMUTABLE | \ MSI_FLAG_MULTI_PCI_MSI) #ifdef CONFIG_PCI_MSI diff --git a/drivers/pci/endpoint/pci-ep-msi.c b/drivers/pci/endpoint/pci-ep-msi.c index b0a91fde202f3..1c157e0a81456 100644 --- a/drivers/pci/endpoint/pci-ep-msi.c +++ b/drivers/pci/endpoint/pci-ep-msi.c @@ -107,6 +107,14 @@ int pci_epf_alloc_doorbell(struct pci_epf *epf, u16 num_db) return -EINVAL; } + if (!dom->msi_parent_ops) + return -EINVAL; + + if (!(dom->msi_parent_ops->supported_flags & MSI_FLAG_MSG_IMMUTABLE)) { + dev_err(dev, "Can't support mutable address/data pair MSI controller\n"); + return -EINVAL; + } + dev_set_msi_domain(dev, dom); msg = kcalloc(num_db, sizeof(struct pci_epf_doorbell_msg), GFP_KERNEL); diff --git a/include/linux/msi.h b/include/linux/msi.h index b10093c4d00ea..d95d979e4747a 100644 --- a/include/linux/msi.h +++ b/include/linux/msi.h @@ -556,6 +556,8 @@ enum { MSI_FLAG_PCI_MSIX_ALLOC_DYN = (1 << 20), /* PCI MSIs cannot be steered separately to CPU cores */ MSI_FLAG_NO_AFFINITY = (1 << 21), + /* Address and data pair is mutable when irq_set_affinity() */ + MSI_FLAG_MSG_IMMUTABLE = (1 << 22), }; /**
Some MSI controller change address/data pair when irq_set_affinity(). Current PCI endpoint can't support this type MSI controller. So add flag MSI_FLAG_MSG_IMMUTABLE in include/linux/msi.h, set this flags in ARM GIC ITS MSI controller and check it when allocate doorbell. Signed-off-by: Frank Li <Frank.Li@nxp.com> --- Change from v9 to v11 - new patch --- drivers/irqchip/irq-gic-v3-its-msi-parent.c | 1 + drivers/pci/endpoint/pci-ep-msi.c | 8 ++++++++ include/linux/msi.h | 2 ++ 3 files changed, 11 insertions(+)