linux-crypto.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* crypto/cavium MSI-X fixups
@ 2017-02-15  7:18 Christoph Hellwig
  2017-02-15  7:18 ` [PATCH 1/3] crypto: cavium: remove dead MSI-X related define Christoph Hellwig
                   ` (4 more replies)
  0 siblings, 5 replies; 10+ messages in thread
From: Christoph Hellwig @ 2017-02-15  7:18 UTC (permalink / raw)
  To: George Cherian; +Cc: David Daney, Herbert Xu, linux-crypto, linux-kernel

Hi George,

your commit "crypto: cavium - Add Support for Octeon-tx CPT Engine"
add a new caller to pci_enable_msix.  This API has long been deprecated
so this series switches it to use pci_alloc_irq_vectors instead.

Can you please test it and make sure it goes in before the end of the
merge window so that no more users of the old API hit mainline?

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/3] crypto: cavium: remove dead MSI-X related define
  2017-02-15  7:18 crypto/cavium MSI-X fixups Christoph Hellwig
@ 2017-02-15  7:18 ` Christoph Hellwig
  2017-02-15  7:18 ` [PATCH 2/3] crypto: cavium/cptpf: switch to pci_alloc_irq_vectors Christoph Hellwig
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Christoph Hellwig @ 2017-02-15  7:18 UTC (permalink / raw)
  To: George Cherian; +Cc: David Daney, Herbert Xu, linux-crypto, linux-kernel

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/crypto/cavium/cpt/cpt_common.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/crypto/cavium/cpt/cpt_common.h b/drivers/crypto/cavium/cpt/cpt_common.h
index ede612f306d3..225078d03773 100644
--- a/drivers/crypto/cavium/cpt/cpt_common.h
+++ b/drivers/crypto/cavium/cpt/cpt_common.h
@@ -20,12 +20,10 @@
 #define CPT_81XX_PCI_VF_DEVICE_ID 0xa041
 
 /* flags to indicate the features supported */
-#define CPT_FLAG_MSIX_ENABLED BIT(0)
 #define CPT_FLAG_SRIOV_ENABLED BIT(1)
 #define CPT_FLAG_VF_DRIVER BIT(2)
 #define CPT_FLAG_DEVICE_READY BIT(3)
 
-#define cpt_msix_enabled(cpt) ((cpt)->flags & CPT_FLAG_MSIX_ENABLED)
 #define cpt_sriov_enabled(cpt) ((cpt)->flags & CPT_FLAG_SRIOV_ENABLED)
 #define cpt_vf_driver(cpt) ((cpt)->flags & CPT_FLAG_VF_DRIVER)
 #define cpt_device_ready(cpt) ((cpt)->flags & CPT_FLAG_DEVICE_READY)
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/3] crypto: cavium/cptpf: switch to pci_alloc_irq_vectors
  2017-02-15  7:18 crypto/cavium MSI-X fixups Christoph Hellwig
  2017-02-15  7:18 ` [PATCH 1/3] crypto: cavium: remove dead MSI-X related define Christoph Hellwig
@ 2017-02-15  7:18 ` Christoph Hellwig
  2017-02-15  7:18 ` [PATCH 3/3] crypto: cavium/cptvf: " Christoph Hellwig
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 10+ messages in thread
From: Christoph Hellwig @ 2017-02-15  7:18 UTC (permalink / raw)
  To: George Cherian; +Cc: David Daney, Herbert Xu, linux-crypto, linux-kernel

pci_enable_msix has been long deprecated, but this driver adds a new
instance.  Convert it to pci_alloc_irq_vectors and greatly simplify
the code.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/crypto/cavium/cpt/cptpf.h      |  5 ---
 drivers/crypto/cavium/cpt/cptpf_main.c | 58 ++++++----------------------------
 2 files changed, 10 insertions(+), 53 deletions(-)

diff --git a/drivers/crypto/cavium/cpt/cptpf.h b/drivers/crypto/cavium/cpt/cptpf.h
index 8a2a8e538da4..c0556c5f63c9 100644
--- a/drivers/crypto/cavium/cpt/cptpf.h
+++ b/drivers/crypto/cavium/cpt/cptpf.h
@@ -51,11 +51,6 @@ struct cpt_device {
 	struct cpt_vf_info vfinfo[CPT_MAX_VF_NUM]; /* Per VF info */
 
 	void __iomem *reg_base; /* Register start address */
-	/* MSI-X */
-	u8 num_vec;
-	bool msix_enabled;
-	struct msix_entry msix_entries[CPT_PF_MSIX_VECTORS];
-	bool irq_allocated[CPT_PF_MSIX_VECTORS];
 	struct pci_dev *pdev; /* pci device handle */
 
 	struct microcode mcode[CPT_MAX_CORE_GROUPS];
diff --git a/drivers/crypto/cavium/cpt/cptpf_main.c b/drivers/crypto/cavium/cpt/cptpf_main.c
index 682d57a11a75..4119c40e7c4b 100644
--- a/drivers/crypto/cavium/cpt/cptpf_main.c
+++ b/drivers/crypto/cavium/cpt/cptpf_main.c
@@ -332,26 +332,6 @@ static int cpt_ucode_load(struct cpt_device *cpt)
 	return ret;
 }
 
-static int cpt_enable_msix(struct cpt_device *cpt)
-{
-	int i, ret;
-
-	cpt->num_vec = CPT_PF_MSIX_VECTORS;
-
-	for (i = 0; i < cpt->num_vec; i++)
-		cpt->msix_entries[i].entry = i;
-
-	ret = pci_enable_msix(cpt->pdev, cpt->msix_entries, cpt->num_vec);
-	if (ret) {
-		dev_err(&cpt->pdev->dev, "Request for #%d msix vectors failed\n",
-			cpt->num_vec);
-		return ret;
-	}
-
-	cpt->msix_enabled = 1;
-	return 0;
-}
-
 static irqreturn_t cpt_mbx0_intr_handler(int irq, void *cpt_irq)
 {
 	struct cpt_device *cpt = (struct cpt_device *)cpt_irq;
@@ -361,26 +341,6 @@ static irqreturn_t cpt_mbx0_intr_handler(int irq, void *cpt_irq)
 	return IRQ_HANDLED;
 }
 
-static void cpt_disable_msix(struct cpt_device *cpt)
-{
-	if (cpt->msix_enabled) {
-		pci_disable_msix(cpt->pdev);
-		cpt->msix_enabled = 0;
-		cpt->num_vec = 0;
-	}
-}
-
-static void cpt_free_all_interrupts(struct cpt_device *cpt)
-{
-	int irq;
-
-	for (irq = 0; irq < cpt->num_vec; irq++) {
-		if (cpt->irq_allocated[irq])
-			free_irq(cpt->msix_entries[irq].vector, cpt);
-		cpt->irq_allocated[irq] = false;
-	}
-}
-
 static void cpt_reset(struct cpt_device *cpt)
 {
 	cpt_write_csr64(cpt->reg_base, CPTX_PF_RESET(0), 1);
@@ -506,32 +466,34 @@ static int cpt_register_interrupts(struct cpt_device *cpt)
 	struct device *dev = &cpt->pdev->dev;
 
 	/* Enable MSI-X */
-	ret = cpt_enable_msix(cpt);
-	if (ret)
+	ret = pci_alloc_irq_vectors(cpt->pdev, CPT_PF_MSIX_VECTORS,
+			CPT_PF_MSIX_VECTORS, PCI_IRQ_MSIX);
+	if (ret < 0) {
+		dev_err(&cpt->pdev->dev, "Request for #%d msix vectors failed\n",
+			CPT_PF_MSIX_VECTORS);
 		return ret;
+	}
 
 	/* Register mailbox interrupt handlers */
-	ret = request_irq(cpt->msix_entries[CPT_PF_INT_VEC_E_MBOXX(0)].vector,
+	ret = request_irq(pci_irq_vector(cpt->pdev, CPT_PF_INT_VEC_E_MBOXX(0)),
 			  cpt_mbx0_intr_handler, 0, "CPT Mbox0", cpt);
 	if (ret)
 		goto fail;
 
-	cpt->irq_allocated[CPT_PF_INT_VEC_E_MBOXX(0)] = true;
-
 	/* Enable mailbox interrupt */
 	cpt_enable_mbox_interrupts(cpt);
 	return 0;
 
 fail:
 	dev_err(dev, "Request irq failed\n");
-	cpt_free_all_interrupts(cpt);
+	pci_disable_msix(cpt->pdev);
 	return ret;
 }
 
 static void cpt_unregister_interrupts(struct cpt_device *cpt)
 {
-	cpt_free_all_interrupts(cpt);
-	cpt_disable_msix(cpt);
+	free_irq(pci_irq_vector(cpt->pdev, CPT_PF_INT_VEC_E_MBOXX(0)), cpt);
+	pci_disable_msix(cpt->pdev);
 }
 
 static int cpt_sriov_init(struct cpt_device *cpt, int num_vfs)
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/3] crypto: cavium/cptvf: switch to pci_alloc_irq_vectors
  2017-02-15  7:18 crypto/cavium MSI-X fixups Christoph Hellwig
  2017-02-15  7:18 ` [PATCH 1/3] crypto: cavium: remove dead MSI-X related define Christoph Hellwig
  2017-02-15  7:18 ` [PATCH 2/3] crypto: cavium/cptpf: switch to pci_alloc_irq_vectors Christoph Hellwig
@ 2017-02-15  7:18 ` Christoph Hellwig
  2017-02-15  9:17 ` crypto/cavium MSI-X fixups George Cherian
  2017-02-23 12:16 ` Herbert Xu
  4 siblings, 0 replies; 10+ messages in thread
From: Christoph Hellwig @ 2017-02-15  7:18 UTC (permalink / raw)
  To: George Cherian; +Cc: David Daney, Herbert Xu, linux-crypto, linux-kernel

pci_enable_msix has been long deprecated, but this driver adds a new
instance.  Convert it to pci_alloc_irq_vectors and greatly simplify
the code, and make sure the prope code properly unwinds.

Signed-off-by: Christoph Hellwig <hch@lst.de>
---
 drivers/crypto/cavium/cpt/cptvf.h      |   3 -
 drivers/crypto/cavium/cpt/cptvf_main.c | 203 +++++++++++----------------------
 2 files changed, 65 insertions(+), 141 deletions(-)

diff --git a/drivers/crypto/cavium/cpt/cptvf.h b/drivers/crypto/cavium/cpt/cptvf.h
index 1cc04aa611e4..0a835a07d4f2 100644
--- a/drivers/crypto/cavium/cpt/cptvf.h
+++ b/drivers/crypto/cavium/cpt/cptvf.h
@@ -107,9 +107,6 @@ struct cpt_vf {
 	void __iomem *reg_base; /* Register start address */
 	void *wqe_info;	/* BH worker info */
 	/* MSI-X */
-	bool msix_enabled;
-	struct msix_entry msix_entries[CPT_VF_MSIX_VECTORS];
-	bool irq_allocated[CPT_VF_MSIX_VECTORS];
 	cpumask_var_t affinity_mask[CPT_VF_MSIX_VECTORS];
 	/* Command and Pending queues */
 	u32 qsize;
diff --git a/drivers/crypto/cavium/cpt/cptvf_main.c b/drivers/crypto/cavium/cpt/cptvf_main.c
index 527bdc3c2969..aac2966ff8d9 100644
--- a/drivers/crypto/cavium/cpt/cptvf_main.c
+++ b/drivers/crypto/cavium/cpt/cptvf_main.c
@@ -357,48 +357,10 @@ static int cptvf_sw_init(struct cpt_vf *cptvf, u32 qlen, u32 nr_queues)
 	return ret;
 }
 
-static void cptvf_disable_msix(struct cpt_vf *cptvf)
+static void cptvf_free_irq_affinity(struct cpt_vf *cptvf, int vec)
 {
-	if (cptvf->msix_enabled) {
-		pci_disable_msix(cptvf->pdev);
-		cptvf->msix_enabled = 0;
-	}
-}
-
-static int cptvf_enable_msix(struct cpt_vf *cptvf)
-{
-	int i, ret;
-
-	for (i = 0; i < CPT_VF_MSIX_VECTORS; i++)
-		cptvf->msix_entries[i].entry = i;
-
-	ret = pci_enable_msix(cptvf->pdev, cptvf->msix_entries,
-			      CPT_VF_MSIX_VECTORS);
-	if (ret) {
-		dev_err(&cptvf->pdev->dev, "Request for #%d msix vectors failed\n",
-			CPT_VF_MSIX_VECTORS);
-		return ret;
-	}
-
-	cptvf->msix_enabled = 1;
-	/* Mark MSIX enabled */
-	cptvf->flags |= CPT_FLAG_MSIX_ENABLED;
-
-	return 0;
-}
-
-static void cptvf_free_all_interrupts(struct cpt_vf *cptvf)
-{
-	int irq;
-
-	for (irq = 0; irq < CPT_VF_MSIX_VECTORS; irq++) {
-		if (cptvf->irq_allocated[irq])
-			irq_set_affinity_hint(cptvf->msix_entries[irq].vector,
-					      NULL);
-		free_cpumask_var(cptvf->affinity_mask[irq]);
-		free_irq(cptvf->msix_entries[irq].vector, cptvf);
-		cptvf->irq_allocated[irq] = false;
-	}
+	irq_set_affinity_hint(pci_irq_vector(cptvf->pdev, vec), NULL);
+	free_cpumask_var(cptvf->affinity_mask[vec]);
 }
 
 static void cptvf_write_vq_ctl(struct cpt_vf *cptvf, bool val)
@@ -650,85 +612,23 @@ static irqreturn_t cptvf_done_intr_handler(int irq, void *cptvf_irq)
 	return IRQ_HANDLED;
 }
 
-static int cptvf_register_misc_intr(struct cpt_vf *cptvf)
-{
-	struct pci_dev *pdev = cptvf->pdev;
-	int ret;
-
-	/* Register misc interrupt handlers */
-	ret = request_irq(cptvf->msix_entries[CPT_VF_INT_VEC_E_MISC].vector,
-			  cptvf_misc_intr_handler, 0, "CPT VF misc intr",
-			  cptvf);
-	if (ret)
-		goto fail;
-
-	cptvf->irq_allocated[CPT_VF_INT_VEC_E_MISC] = true;
-
-	/* Enable mailbox interrupt */
-	cptvf_enable_mbox_interrupts(cptvf);
-	cptvf_enable_swerr_interrupts(cptvf);
-
-	return 0;
-
-fail:
-	dev_err(&pdev->dev, "Request misc irq failed");
-	cptvf_free_all_interrupts(cptvf);
-	return ret;
-}
-
-static int cptvf_register_done_intr(struct cpt_vf *cptvf)
-{
-	struct pci_dev *pdev = cptvf->pdev;
-	int ret;
-
-	/* Register DONE interrupt handlers */
-	ret = request_irq(cptvf->msix_entries[CPT_VF_INT_VEC_E_DONE].vector,
-			  cptvf_done_intr_handler, 0, "CPT VF done intr",
-			  cptvf);
-	if (ret)
-		goto fail;
-
-	cptvf->irq_allocated[CPT_VF_INT_VEC_E_DONE] = true;
-
-	/* Enable mailbox interrupt */
-	cptvf_enable_done_interrupts(cptvf);
-	return 0;
-
-fail:
-	dev_err(&pdev->dev, "Request done irq failed\n");
-	cptvf_free_all_interrupts(cptvf);
-	return ret;
-}
-
-static void cptvf_unregister_interrupts(struct cpt_vf *cptvf)
-{
-	cptvf_free_all_interrupts(cptvf);
-	cptvf_disable_msix(cptvf);
-}
-
-static void cptvf_set_irq_affinity(struct cpt_vf *cptvf)
+static void cptvf_set_irq_affinity(struct cpt_vf *cptvf, int vec)
 {
 	struct pci_dev *pdev = cptvf->pdev;
-	int vec, cpu;
-	int irqnum;
-
-	for (vec = 0; vec < CPT_VF_MSIX_VECTORS; vec++) {
-		if (!cptvf->irq_allocated[vec])
-			continue;
-
-		if (!zalloc_cpumask_var(&cptvf->affinity_mask[vec],
-					GFP_KERNEL)) {
-			dev_err(&pdev->dev, "Allocation failed for affinity_mask for VF %d",
-				cptvf->vfid);
-			return;
-		}
+	int cpu;
 
-		cpu = cptvf->vfid % num_online_cpus();
-		cpumask_set_cpu(cpumask_local_spread(cpu, cptvf->node),
-				cptvf->affinity_mask[vec]);
-		irqnum = cptvf->msix_entries[vec].vector;
-		irq_set_affinity_hint(irqnum, cptvf->affinity_mask[vec]);
+	if (!zalloc_cpumask_var(&cptvf->affinity_mask[vec],
+				GFP_KERNEL)) {
+		dev_err(&pdev->dev, "Allocation failed for affinity_mask for VF %d",
+			cptvf->vfid);
+		return;
 	}
+
+	cpu = cptvf->vfid % num_online_cpus();
+	cpumask_set_cpu(cpumask_local_spread(cpu, cptvf->node),
+			cptvf->affinity_mask[vec]);
+	irq_set_affinity_hint(pci_irq_vector(pdev, vec),
+			cptvf->affinity_mask[vec]);
 }
 
 static void cptvf_write_vq_saddr(struct cpt_vf *cptvf, u64 val)
@@ -809,22 +709,32 @@ static int cptvf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	}
 
 	cptvf->node = dev_to_node(&pdev->dev);
-	/* Enable MSI-X */
-	err = cptvf_enable_msix(cptvf);
-	if (err) {
-		dev_err(dev, "cptvf_enable_msix() failed");
+	err = pci_alloc_irq_vectors(pdev, CPT_VF_MSIX_VECTORS,
+			CPT_VF_MSIX_VECTORS, PCI_IRQ_MSIX);
+	if (err < 0) {
+		dev_err(dev, "Request for #%d msix vectors failed\n",
+			CPT_VF_MSIX_VECTORS);
 		goto cptvf_err_release_regions;
 	}
 
-	/* Register mailbox interrupts */
-	cptvf_register_misc_intr(cptvf);
+	err = request_irq(pci_irq_vector(pdev, CPT_VF_INT_VEC_E_MISC),
+			  cptvf_misc_intr_handler, 0, "CPT VF misc intr",
+			  cptvf);
+	if (err) {
+		dev_err(dev, "Request misc irq failed");
+		goto cptvf_free_vectors;
+	}
+
+	/* Enable mailbox interrupt */
+	cptvf_enable_mbox_interrupts(cptvf);
+	cptvf_enable_swerr_interrupts(cptvf);
 
 	/* Check ready with PF */
 	/* Gets chip ID / device Id from PF if ready */
 	err = cptvf_check_pf_ready(cptvf);
 	if (err) {
 		dev_err(dev, "PF not responding to READY msg");
-		goto cptvf_err_release_regions;
+		goto cptvf_free_misc_irq;
 	}
 
 	/* CPT VF software resources initialization */
@@ -832,13 +742,13 @@ static int cptvf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	err = cptvf_sw_init(cptvf, CPT_CMD_QLEN, CPT_NUM_QS_PER_VF);
 	if (err) {
 		dev_err(dev, "cptvf_sw_init() failed");
-		goto cptvf_err_release_regions;
+		goto cptvf_free_misc_irq;
 	}
 	/* Convey VQ LEN to PF */
 	err = cptvf_send_vq_size_msg(cptvf);
 	if (err) {
 		dev_err(dev, "PF not responding to QLEN msg");
-		goto cptvf_err_release_regions;
+		goto cptvf_free_misc_irq;
 	}
 
 	/* CPT VF device initialization */
@@ -848,37 +758,50 @@ static int cptvf_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	err = cptvf_send_vf_to_grp_msg(cptvf);
 	if (err) {
 		dev_err(dev, "PF not responding to VF_GRP msg");
-		goto cptvf_err_release_regions;
+		goto cptvf_free_misc_irq;
 	}
 
 	cptvf->priority = 1;
 	err = cptvf_send_vf_priority_msg(cptvf);
 	if (err) {
 		dev_err(dev, "PF not responding to VF_PRIO msg");
-		goto cptvf_err_release_regions;
+		goto cptvf_free_misc_irq;
+	}
+
+	err = request_irq(pci_irq_vector(pdev, CPT_VF_INT_VEC_E_DONE),
+			  cptvf_done_intr_handler, 0, "CPT VF done intr",
+			  cptvf);
+	if (err) {
+		dev_err(dev, "Request done irq failed\n");
+		goto cptvf_free_misc_irq;
 	}
-	/* Register DONE interrupts */
-	err = cptvf_register_done_intr(cptvf);
-	if (err)
-		goto cptvf_err_release_regions;
+
+	/* Enable mailbox interrupt */
+	cptvf_enable_done_interrupts(cptvf);
 
 	/* Set irq affinity masks */
-	cptvf_set_irq_affinity(cptvf);
-	/* Convey UP to PF */
+	cptvf_set_irq_affinity(cptvf, CPT_VF_INT_VEC_E_MISC);
+	cptvf_set_irq_affinity(cptvf, CPT_VF_INT_VEC_E_DONE);
+
 	err = cptvf_send_vf_up(cptvf);
 	if (err) {
 		dev_err(dev, "PF not responding to UP msg");
-		goto cptvf_up_fail;
+		goto cptvf_free_irq_affinity;
 	}
 	err = cvm_crypto_init(cptvf);
 	if (err) {
 		dev_err(dev, "Algorithm register failed\n");
-		goto cptvf_up_fail;
+		goto cptvf_free_irq_affinity;
 	}
 	return 0;
 
-cptvf_up_fail:
-	cptvf_unregister_interrupts(cptvf);
+cptvf_free_irq_affinity:
+	cptvf_free_irq_affinity(cptvf, CPT_VF_INT_VEC_E_DONE);
+	cptvf_free_irq_affinity(cptvf, CPT_VF_INT_VEC_E_MISC);
+cptvf_free_misc_irq:
+	free_irq(pci_irq_vector(pdev, CPT_VF_INT_VEC_E_MISC), cptvf);
+cptvf_free_vectors:
+	pci_free_irq_vectors(cptvf->pdev);
 cptvf_err_release_regions:
 	pci_release_regions(pdev);
 cptvf_err_disable_device:
@@ -899,7 +822,11 @@ static void cptvf_remove(struct pci_dev *pdev)
 	if (cptvf_send_vf_down(cptvf)) {
 		dev_err(&pdev->dev, "PF not responding to DOWN msg");
 	} else {
-		cptvf_unregister_interrupts(cptvf);
+		cptvf_free_irq_affinity(cptvf, CPT_VF_INT_VEC_E_DONE);
+		cptvf_free_irq_affinity(cptvf, CPT_VF_INT_VEC_E_MISC);
+		free_irq(pci_irq_vector(pdev, CPT_VF_INT_VEC_E_DONE), cptvf);
+		free_irq(pci_irq_vector(pdev, CPT_VF_INT_VEC_E_MISC), cptvf);
+		pci_free_irq_vectors(cptvf->pdev);
 		cptvf_sw_cleanup(cptvf);
 		pci_set_drvdata(pdev, NULL);
 		pci_release_regions(pdev);
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: crypto/cavium MSI-X fixups
  2017-02-15  7:18 crypto/cavium MSI-X fixups Christoph Hellwig
                   ` (2 preceding siblings ...)
  2017-02-15  7:18 ` [PATCH 3/3] crypto: cavium/cptvf: " Christoph Hellwig
@ 2017-02-15  9:17 ` George Cherian
  2017-02-19 17:32   ` Christoph Hellwig
  2017-02-23 12:16 ` Herbert Xu
  4 siblings, 1 reply; 10+ messages in thread
From: George Cherian @ 2017-02-15  9:17 UTC (permalink / raw)
  To: Christoph Hellwig, George Cherian
  Cc: David Daney, Herbert Xu, linux-crypto, linux-kernel

Hi Christoph,


On 02/15/2017 12:48 PM, Christoph Hellwig wrote:
> Hi George,
>
> your commit "crypto: cavium - Add Support for Octeon-tx CPT Engine"
> add a new caller to pci_enable_msix.  This API has long been deprecated
> so this series switches it to use pci_alloc_irq_vectors instead.
>
> Can you please test it and make sure it goes in before the end of the
> merge window so that no more users of the old API hit mainline?

Yes the changes works well.
Acked-by: George Cherian <george.cherian@cavium.com>

for the series.
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: crypto/cavium MSI-X fixups
  2017-02-15  9:17 ` crypto/cavium MSI-X fixups George Cherian
@ 2017-02-19 17:32   ` Christoph Hellwig
  2017-02-21 17:36     ` David Daney
  0 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2017-02-19 17:32 UTC (permalink / raw)
  To: George Cherian
  Cc: Christoph Hellwig, George Cherian, David Daney, Herbert Xu,
	linux-crypto, linux-kernel

Herbert,

any comment?  I'd really like to avoid introducing new pci_enable_msix
users in this merge window..

On Wed, Feb 15, 2017 at 02:47:09PM +0530, George Cherian wrote:
> Hi Christoph,
>
>
> On 02/15/2017 12:48 PM, Christoph Hellwig wrote:
>> Hi George,
>>
>> your commit "crypto: cavium - Add Support for Octeon-tx CPT Engine"
>> add a new caller to pci_enable_msix.  This API has long been deprecated
>> so this series switches it to use pci_alloc_irq_vectors instead.
>>
>> Can you please test it and make sure it goes in before the end of the
>> merge window so that no more users of the old API hit mainline?
>
> Yes the changes works well.
> Acked-by: George Cherian <george.cherian@cavium.com>
>
> for the series.
>>
---end quoted text---

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: crypto/cavium MSI-X fixups
  2017-02-19 17:32   ` Christoph Hellwig
@ 2017-02-21 17:36     ` David Daney
  2017-02-22  7:27       ` Christoph Hellwig
  0 siblings, 1 reply; 10+ messages in thread
From: David Daney @ 2017-02-21 17:36 UTC (permalink / raw)
  To: Christoph Hellwig, George Cherian
  Cc: George Cherian, David Daney, Herbert Xu, linux-crypto, linux-kernel

On 02/19/2017 09:32 AM, Christoph Hellwig wrote:
> Herbert,
>
> any comment?  I'd really like to avoid introducing new pci_enable_msix
> users in this merge window..

Hi Cristoph,

With respect to pci_enable_msix(), what do you recommend as a 
replacement?  For the crypto/cavium driver, you recommend 
pci_alloc_irq_vectors(), which works well if the required MSI-X indexes 
are contiguous starting at zero.   What would be used for a device that 
has 184 MSI-X, but only a sparse subset (fewer than half) of these are 
required for the driver operation.  It would waste system resources to 
use an API that forces us to allocate 184 when only 80 are required.

Currently pci_enable_msix() allows an arbitrary set of MSI-X to be 
requested, which exactly fits the requirements of our (non 
crypto/cavium) hardware.

Thanks in advance for any insight you can provide,
David Daney


>
> On Wed, Feb 15, 2017 at 02:47:09PM +0530, George Cherian wrote:
>> Hi Christoph,
>>
>>
>> On 02/15/2017 12:48 PM, Christoph Hellwig wrote:
>>> Hi George,
>>>
>>> your commit "crypto: cavium - Add Support for Octeon-tx CPT Engine"
>>> add a new caller to pci_enable_msix.  This API has long been deprecated
>>> so this series switches it to use pci_alloc_irq_vectors instead.
>>>
>>> Can you please test it and make sure it goes in before the end of the
>>> merge window so that no more users of the old API hit mainline?
>>
>> Yes the changes works well.
>> Acked-by: George Cherian <george.cherian@cavium.com>
>>
>> for the series.
>>>
> ---end quoted text---
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: crypto/cavium MSI-X fixups
  2017-02-21 17:36     ` David Daney
@ 2017-02-22  7:27       ` Christoph Hellwig
  2017-02-22 17:45         ` David Daney
  0 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2017-02-22  7:27 UTC (permalink / raw)
  To: David Daney
  Cc: Christoph Hellwig, George Cherian, George Cherian, David Daney,
	Herbert Xu, linux-crypto, linux-kernel

On Tue, Feb 21, 2017 at 09:36:04AM -0800, David Daney wrote:
> With respect to pci_enable_msix(), what do you recommend as a replacement?  

pci_alloc_irq_vectors.  In fact I have a tree ready for after -rc1
that removes pci_enable_msix() entirely.

> For the crypto/cavium driver, you recommend pci_alloc_irq_vectors(), which 
> works well if the required MSI-X indexes are contiguous starting at zero.   
> What would be used for a device that has 184 MSI-X, but only a sparse 
> subset (fewer than half) of these are required for the driver operation.  
> It would waste system resources to use an API that forces us to allocate 
> 184 when only 80 are required.

Currently we don't have a good API for that.  I've not been through all
users of pci_enable_msix_{range,exact} yet, but so far I've only found
one user not using all vectors from 0 to some limit.  Depending how many
such users we have and how they'll look I will have to look into an API
to support that use case.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: crypto/cavium MSI-X fixups
  2017-02-22  7:27       ` Christoph Hellwig
@ 2017-02-22 17:45         ` David Daney
  0 siblings, 0 replies; 10+ messages in thread
From: David Daney @ 2017-02-22 17:45 UTC (permalink / raw)
  To: Christoph Hellwig, David Daney
  Cc: George Cherian, George Cherian, David Daney, Herbert Xu,
	linux-crypto, linux-kernel

On 02/21/2017 11:27 PM, Christoph Hellwig wrote:
> On Tue, Feb 21, 2017 at 09:36:04AM -0800, David Daney wrote:
>> With respect to pci_enable_msix(), what do you recommend as a replacement?
>
> pci_alloc_irq_vectors.  In fact I have a tree ready for after -rc1
> that removes pci_enable_msix() entirely.

I would like to see something that handles the case of this driver that 
I am attempting to get merged:


https://lkml.org/lkml/2017/2/15/1209


>
>> For the crypto/cavium driver, you recommend pci_alloc_irq_vectors(), which
>> works well if the required MSI-X indexes are contiguous starting at zero.
>> What would be used for a device that has 184 MSI-X, but only a sparse
>> subset (fewer than half) of these are required for the driver operation.
>> It would waste system resources to use an API that forces us to allocate
>> 184 when only 80 are required.
>
> Currently we don't have a good API for that.  I've not been through all
> users of pci_enable_msix_{range,exact} yet, but so far I've only found
> one user not using all vectors from 0 to some limit.  Depending how many
> such users we have and how they'll look I will have to look into an API
> to support that use case.
>

See the patch above for the case that doesn't use 0..UPPER_LIMIT

David Daney

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: crypto/cavium MSI-X fixups
  2017-02-15  7:18 crypto/cavium MSI-X fixups Christoph Hellwig
                   ` (3 preceding siblings ...)
  2017-02-15  9:17 ` crypto/cavium MSI-X fixups George Cherian
@ 2017-02-23 12:16 ` Herbert Xu
  4 siblings, 0 replies; 10+ messages in thread
From: Herbert Xu @ 2017-02-23 12:16 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: George Cherian, David Daney, linux-crypto, linux-kernel

On Wed, Feb 15, 2017 at 08:18:40AM +0100, Christoph Hellwig wrote:
> Hi George,
> 
> your commit "crypto: cavium - Add Support for Octeon-tx CPT Engine"
> add a new caller to pci_enable_msix.  This API has long been deprecated
> so this series switches it to use pci_alloc_irq_vectors instead.
> 
> Can you please test it and make sure it goes in before the end of the
> merge window so that no more users of the old API hit mainline?

All applied.  Thanks.
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-02-23 12:17 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-15  7:18 crypto/cavium MSI-X fixups Christoph Hellwig
2017-02-15  7:18 ` [PATCH 1/3] crypto: cavium: remove dead MSI-X related define Christoph Hellwig
2017-02-15  7:18 ` [PATCH 2/3] crypto: cavium/cptpf: switch to pci_alloc_irq_vectors Christoph Hellwig
2017-02-15  7:18 ` [PATCH 3/3] crypto: cavium/cptvf: " Christoph Hellwig
2017-02-15  9:17 ` crypto/cavium MSI-X fixups George Cherian
2017-02-19 17:32   ` Christoph Hellwig
2017-02-21 17:36     ` David Daney
2017-02-22  7:27       ` Christoph Hellwig
2017-02-22 17:45         ` David Daney
2017-02-23 12:16 ` Herbert Xu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).