Re: [PATCH v11 4/8] cxl/pci: Create PCI DOE mailbox's for memory devices
From: Davidlohr Bueso
Date: Fri Jun 17 2022 - 16:43:08 EST
On Fri, 10 Jun 2022, ira.weiny@xxxxxxxxx wrote:
+++ b/drivers/cxl/cxlmem.h
@@ -191,6 +191,8 @@ struct cxl_endpoint_dvsec_info {
* @component_reg_phys: register base of component registers
* @info: Cached DVSEC information about the device.
* @serial: PCIe Device Serial Number
Missing doc:
@doe_use_irq: Use interrupt vectors for DOEs over polling.
However introducing such flags is not pretty, and this is only used by
devm_cxl_pci_create_doe(). Do we really need it? See below.
+ * @doe_mbs: PCI DOE mailbox array
+ * @num_mbs: Number of DOE mailboxes
* @mbox_send: @dev specific transport for transmitting mailbox commands
*
* See section 8.2.9.5.2 Capacity Configuration and Label Storage for
@@ -224,6 +226,10 @@ struct cxl_dev_state {
resource_size_t component_reg_phys;
u64 serial;
+ bool doe_use_irq;
+ struct pci_doe_mb **doe_mbs;
+ int num_mbs;
+
int (*mbox_send)(struct cxl_dev_state *cxlds, struct cxl_mbox_cmd *cmd);
};
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 5a0ae46d4989..72c7b535f5df 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -8,6 +8,7 @@
#include <linux/mutex.h>
#include <linux/list.h>
#include <linux/pci.h>
+#include <linux/pci-doe.h>
#include <linux/io.h>
#include "cxlmem.h"
#include "cxlpci.h"
@@ -386,6 +387,116 @@ static int cxl_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type,
return rc;
}
+static void cxl_pci_free_irq_vectors(void *data)
+{
+ pci_free_irq_vectors(data);
+}
+
+static void cxl_doe_destroy_mb(void *ds)
+{
+ struct cxl_dev_state *cxlds = ds;
+ int i;
+
+ for (i = 0; i < cxlds->num_mbs; i++) {
+ if (cxlds->doe_mbs[i])
+ pci_doe_destroy_mb(cxlds->doe_mbs[i]);
+ }
+}
+
+static void cxl_alloc_irq_vectors(struct cxl_dev_state *cxlds)
+{
+ struct device *dev = cxlds->dev;
+ struct pci_dev *pdev = to_pci_dev(dev);
+ int max_irqs = 0;
+ int off = 0;
+ int rc;
+
+ /* Account for all the DOE vectors needed */
+ pci_doe_for_each_off(pdev, off) {
+ int irq = pci_doe_get_irq_num(pdev, off);
+
+ if (irq < 0)
+ continue;
+ max_irqs = max(max_irqs, irq + 1);
+ }
+
+ if (!max_irqs)
+ return;
+
+ cxlds->doe_use_irq = false;
Is this unnecessary, it should already be 0 per the devm_kzalloc().
+
+ /*
+ * Allocate enough vectors for the DOE's
+ */
+ rc = pci_alloc_irq_vectors(pdev, max_irqs, max_irqs, PCI_IRQ_MSI |
+ PCI_IRQ_MSIX);
+ if (rc != max_irqs) {
+ pci_err(pdev, "Not enough interrupts; use polling\n");
+ /* Some got allocated; clean them up */
+ if (rc > 0)
+ cxl_pci_free_irq_vectors(pdev);
+ return;
+ }
+
+ rc = devm_add_action_or_reset(dev, cxl_pci_free_irq_vectors, pdev);
+ if (rc)
+ return;
+
+ cxlds->doe_use_irq = true;
+}
+
+/**
+ * devm_cxl_pci_create_doe - Scan and set up DOE mailboxes
+ *
+ * @cxlds: The CXL device state
+ */
+static void devm_cxl_pci_create_doe(struct cxl_dev_state *cxlds)
+{
+ struct device *dev = cxlds->dev;
+ struct pci_dev *pdev = to_pci_dev(dev);
+ u16 off = 0;
+ int num_mbs = 0;
+ int rc;
+
+ pci_doe_for_each_off(pdev, off)
+ num_mbs++;
+
+ if (!num_mbs) {
+ pci_dbg(pdev, "0 DOE mailbox's found\n");
+ return;
+ }
+
+ cxlds->doe_mbs = devm_kcalloc(dev, num_mbs, sizeof(*cxlds->doe_mbs),
+ GFP_KERNEL);
+ if (!cxlds->doe_mbs)
+ return;
+
+ pci_doe_for_each_off(pdev, off) {
+ struct pci_doe_mb *doe_mb;
+ int irq = -1;
+
+ if (cxlds->doe_use_irq)
+ irq = pci_doe_get_irq_num(pdev, off);
+
+ doe_mb = pci_doe_create_mb(pdev, off, irq);
+ if (IS_ERR(doe_mb)) {
+ pci_err(pdev,
+ "Failed to create MB object for MB @ %x\n",
+ off);
+ doe_mb = NULL;
+ }
+
+ cxlds->doe_mbs[cxlds->num_mbs] = doe_mb;
+ cxlds->num_mbs++;
+ }
+
+ rc = devm_add_action_or_reset(dev, cxl_doe_destroy_mb, cxlds);
+ if (rc)
+ return;
+
+ pci_info(pdev, "Configured %d DOE mailbox's\n", cxlds->num_mbs);
+}
+
static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
{
struct cxl_register_map map;
@@ -434,6 +545,9 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
cxlds->component_reg_phys = cxl_regmap_to_base(pdev, &map);
+ cxl_alloc_irq_vectors(cxlds);
+ devm_cxl_pci_create_doe(cxlds);
Should cxl_alloc_irq_vectors() just be called directly from devm_cxl_pci_create_doe()
instead? Also if devm_cxl_pci_create_doe() fails (say ENOMEM), why do we
bother continuing the cxl_pci probing?
Thanks,
Davidlohr
------
diff --git a/drivers/cxl/cxlmem.h b/drivers/cxl/cxlmem.h
index ce5b00f3ebcb..44098c785a8b 100644
--- a/drivers/cxl/cxlmem.h
+++ b/drivers/cxl/cxlmem.h
@@ -230,7 +230,6 @@ struct cxl_dev_state {
resource_size_t component_reg_phys;
u64 serial;
- bool doe_use_irq;
struct pci_doe_mb **doe_mbs;
int num_mbs;
diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
index 72c7b535f5df..47c3741f7768 100644
--- a/drivers/cxl/pci.c
+++ b/drivers/cxl/pci.c
@@ -403,7 +403,7 @@ static void cxl_doe_destroy_mb(void *ds)
}
}
-static void cxl_alloc_irq_vectors(struct cxl_dev_state *cxlds)
+static int cxl_alloc_irq_vectors(struct cxl_dev_state *cxlds)
{
struct device *dev = cxlds->dev;
struct pci_dev *pdev = to_pci_dev(dev);
@@ -421,9 +421,7 @@ static void cxl_alloc_irq_vectors(struct cxl_dev_state *cxlds)
}
if (!max_irqs)
- return;
-
- cxlds->doe_use_irq = false;
+ return -ENOMEM;
/*
* Allocate enough vectors for the DOE's
@@ -435,14 +433,10 @@ static void cxl_alloc_irq_vectors(struct cxl_dev_state *cxlds)
/* Some got allocated; clean them up */
if (rc > 0)
cxl_pci_free_irq_vectors(pdev);
- return;
+ return -ENOMEM;
}
- rc = devm_add_action_or_reset(dev, cxl_pci_free_irq_vectors, pdev);
- if (rc)
- return;
-
- cxlds->doe_use_irq = true;
+ return devm_add_action_or_reset(dev, cxl_pci_free_irq_vectors, pdev);
}
/**
@@ -457,6 +451,10 @@ static void devm_cxl_pci_create_doe(struct cxl_dev_state *cxlds)
u16 off = 0;
int num_mbs = 0;
int rc;
+ bool doe_use_irq = false;
+
+ if (cxl_alloc_irq_vectors(cxlds))
+ doe_use_irq = true;
pci_doe_for_each_off(pdev, off)
num_mbs++;
@@ -475,7 +473,7 @@ static void devm_cxl_pci_create_doe(struct cxl_dev_state *cxlds)
struct pci_doe_mb *doe_mb;
int irq = -1;
- if (cxlds->doe_use_irq)
+ if (doe_use_irq)
irq = pci_doe_get_irq_num(pdev, off);
doe_mb = pci_doe_create_mb(pdev, off, irq);
@@ -545,7 +543,6 @@ static int cxl_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id)
cxlds->component_reg_phys = cxl_regmap_to_base(pdev, &map);
- cxl_alloc_irq_vectors(cxlds);
devm_cxl_pci_create_doe(cxlds);
rc = cxl_pci_setup_mailbox(cxlds);