| Message ID | 20241204-ep-msi-v10-3-87c378dbcd6d@nxp.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | PCI: EP: Add RC-to-EP doorbell with platform MSI controller | expand |
On Wed, Dec 04 2024 at 18:25, 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_MUTABLE in include/linux/msi.h and check it when allocate > doorbell. Q: Who is going to annotate the affected domains with that flag? A: Nobody. Q: What's the value of the flag? A: Zero, as as it prevents exactly nothing. You want a MSI_FLAG_MSG_IMMUTABLE and set that on the domains which provide it. That way you ensure that someone looked at the domain to validate it. Thanks, tglx
On Thu, Dec 05, 2024 at 02:10:55PM +0100, Thomas Gleixner wrote: > On Wed, Dec 04 2024 at 18:25, 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_MUTABLE in include/linux/msi.h and check it when allocate > > doorbell. > > Q: Who is going to annotate the affected domains with that flag? > > A: Nobody. > > Q: What's the value of the flag? > > A: Zero, as as it prevents exactly nothing. > > You want a MSI_FLAG_MSG_IMMUTABLE and set that on the domains which > provide it. That way you ensure that someone looked at the domain to > validate it. Okay, at beginning I think most MSI controller is immutable. So I use MSI_FLAG_MSG_MUTABLE. It is fine change to MSI_FLAG_MSG_IMMUTABLE. Frank > > Thanks, > > tglx
On Thu, Dec 05 2024 at 09:56, Frank Li wrote: > On Thu, Dec 05, 2024 at 02:10:55PM +0100, Thomas Gleixner wrote: >> You want a MSI_FLAG_MSG_IMMUTABLE and set that on the domains which >> provide it. That way you ensure that someone looked at the domain to >> validate it. > > Okay, at beginning I think most MSI controller is immutable. So I use > MSI_FLAG_MSG_MUTABLE. If you want to do that then _you_ have to go through every single interrupt controllers, validate and opt-out in case it does change the message. Otherwise that flag is completely pointless. Instead of adding the IMMUTABLE flag for one controller you know and then let others who want to utilize this amend their controllers. Opt-in is less work and more safe than opt-out. See? Thanks, tglx
diff --git a/drivers/pci/endpoint/pci-ep-msi.c b/drivers/pci/endpoint/pci-ep-msi.c index b0a91fde202f3..c4a4f211cb27d 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_MUTABLE) { + 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..e404e0a01692a 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_MUTABLE = (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_MUTABLE in include/linux/msi.h and check it when allocate doorbell. Signed-off-by: Frank Li <Frank.Li@nxp.com> --- Change from v9 to v10 - new patch --- drivers/pci/endpoint/pci-ep-msi.c | 8 ++++++++ include/linux/msi.h | 2 ++ 2 files changed, 10 insertions(+)