| Message ID | 20240117-dont-fail-irq-v2-1-f33f26b0e365@intel.com |
|---|---|
| State | Accepted |
| Commit | d72a4caf685989e353dff0a97d7376ee10edbf87 |
| Headers | show |
| Series | [v2] cxl/pci: Skip irq features if MSI/MSI-X are not supported | expand |
On Wed, 17 Jan 2024, Ira Weiny wrote: >CXL 3.1 Section 3.1.1 states: > > "A Function on a CXL device must not generate INTx messages if > that Function participates in CXL.cache protocol or CXL.mem > protocols." > >The generic CXL memory driver only supports devices which use the >CXL.mem protocol. The current driver attempts to allocate MSI/MSI-X >vectors in anticipation of their need for mailbox interrupts or event >processing. However, the above requirement does not require a device to >support interrupts, only that they use MSI/MSI-X. For example, a device >may disable mailbox interrupts and either be configured for firmware >first or skip event processing and function. > >Dave Larsen reported that the following Intel / Agilex card does not >support interrupts on function 0. > > CXL: Intel Corporation Device 0ddb (rev 01) (prog-if 10 [CXL Memory Device (CXL 2.x)]) > >Rather than fail device probe if interrupts are not supported; flag that >irqs are not enabled and avoid features which require interrupts. >Emit messages appropriate for the situation to aid in debugging should >device behavior be unexpected due to a failure to allocate vectors. > >Note that it is possible for a device to have host based event >processing through polling. However, the driver does not support >polling and it is not anticipated to be generally required. Leave that >functionality to a future patch if such a device comes along. > >Reported-by: Dave Larsen <davelarsen58@gmail.com> >Reviewed-by: Dave Jiang <dave.jiang@intel.com> >Reviewed-by: Fan Ni <fan.ni@samsung.com> >Signed-off-by: Ira Weiny <ira.weiny@intel.com> Yes, I suspected this might come up at some point. Reviewed-and-tested-by: Davidlohr Bueso <dave@stgolabs.net>
Ira Weiny wrote: > CXL 3.1 Section 3.1.1 states: > > "A Function on a CXL device must not generate INTx messages if > that Function participates in CXL.cache protocol or CXL.mem > protocols." > > The generic CXL memory driver only supports devices which use the > CXL.mem protocol. The current driver attempts to allocate MSI/MSI-X > vectors in anticipation of their need for mailbox interrupts or event > processing. However, the above requirement does not require a device to > support interrupts, only that they use MSI/MSI-X. For example, a device > may disable mailbox interrupts and either be configured for firmware > first or skip event processing and function. > > Dave Larsen reported that the following Intel / Agilex card does not > support interrupts on function 0. > > CXL: Intel Corporation Device 0ddb (rev 01) (prog-if 10 [CXL Memory Device (CXL 2.x)]) > > Rather than fail device probe if interrupts are not supported; flag that > irqs are not enabled and avoid features which require interrupts. > Emit messages appropriate for the situation to aid in debugging should > device behavior be unexpected due to a failure to allocate vectors. > > Note that it is possible for a device to have host based event > processing through polling. However, the driver does not support > polling and it is not anticipated to be generally required. Leave that > functionality to a future patch if such a device comes along. > > Reported-by: Dave Larsen <davelarsen58@gmail.com> > Reviewed-by: Dave Jiang <dave.jiang@intel.com> > Reviewed-by: Fan Ni <fan.ni@samsung.com> > Signed-off-by: Ira Weiny <ira.weiny@intel.com> > --- > Changes in v2: > - [djbw: add reported by and info about the card.] > - [djbw: skip error reporting in the mailbox case.] > - [djbw: clean up event message] > - [iweiny: pick up tags] > - Link to v1: https://lore.kernel.org/r/20240111-dont-fail-irq-v1-1-802c22a79ecb@intel.com > --- > drivers/cxl/pci.c | 26 +++++++++++++++----------- > 1 file changed, 15 insertions(+), 11 deletions(-) Looks good, I consider this suitable as post -rc1 material. Will get it queued once the merge window closes.
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c index 0155fb66b580..93df03f0622e 100644 --- a/drivers/cxl/pci.c +++ b/drivers/cxl/pci.c @@ -381,7 +381,7 @@ static int cxl_pci_mbox_send(struct cxl_memdev_state *mds, return rc; } -static int cxl_pci_setup_mailbox(struct cxl_memdev_state *mds) +static int cxl_pci_setup_mailbox(struct cxl_memdev_state *mds, bool irq_avail) { struct cxl_dev_state *cxlds = &mds->cxlds; const int cap = readl(cxlds->regs.mbox + CXLDEV_MBOX_CAPS_OFFSET); @@ -440,7 +440,7 @@ static int cxl_pci_setup_mailbox(struct cxl_memdev_state *mds) INIT_DELAYED_WORK(&mds->security.poll_dwork, cxl_mbox_sanitize_work); /* background command interrupts are optional */ - if (!(cap & CXLDEV_MBOX_CAP_BG_CMD_IRQ)) + if (!(cap & CXLDEV_MBOX_CAP_BG_CMD_IRQ) || !irq_avail) return 0; msgnum = FIELD_GET(CXLDEV_MBOX_CAP_IRQ_MSGNUM_MASK, cap); @@ -587,7 +587,7 @@ static int cxl_mem_alloc_event_buf(struct cxl_memdev_state *mds) return devm_add_action_or_reset(mds->cxlds.dev, free_event_buf, buf); } -static int cxl_alloc_irq_vectors(struct pci_dev *pdev) +static bool cxl_alloc_irq_vectors(struct pci_dev *pdev) { int nvecs; @@ -604,9 +604,9 @@ static int cxl_alloc_irq_vectors(struct pci_dev *pdev) PCI_IRQ_MSIX | PCI_IRQ_MSI); if (nvecs < 1) { dev_dbg(&pdev->dev, "Failed to alloc irq vectors: %d\n", nvecs); - return -ENXIO; + return false; } - return 0; + return true; } static irqreturn_t cxl_event_thread(int irq, void *id) @@ -742,7 +742,7 @@ static bool cxl_event_int_is_fw(u8 setting) } static int cxl_event_config(struct pci_host_bridge *host_bridge, - struct cxl_memdev_state *mds) + struct cxl_memdev_state *mds, bool irq_avail) { struct cxl_event_interrupt_policy policy; int rc; @@ -754,6 +754,11 @@ static int cxl_event_config(struct pci_host_bridge *host_bridge, if (!host_bridge->native_cxl_error) return 0; + if (!irq_avail) { + dev_info(mds->cxlds.dev, "No interrupt support, disable event processing.\n"); + return 0; + } + rc = cxl_mem_alloc_event_buf(mds); if (rc) return rc; @@ -788,6 +793,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) struct cxl_register_map map; struct cxl_memdev *cxlmd; int i, rc, pmu_count; + bool irq_avail; /* * Double check the anonymous union trickery in struct cxl_regs @@ -845,11 +851,9 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) else dev_warn(&pdev->dev, "Media not active (%d)\n", rc); - rc = cxl_alloc_irq_vectors(pdev); - if (rc) - return rc; + irq_avail = cxl_alloc_irq_vectors(pdev); - rc = cxl_pci_setup_mailbox(mds); + rc = cxl_pci_setup_mailbox(mds, irq_avail); if (rc) return rc; @@ -908,7 +912,7 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) } } - rc = cxl_event_config(host_bridge, mds); + rc = cxl_event_config(host_bridge, mds, irq_avail); if (rc) return rc;