[PATCH 0/4] fnic: fixup INTx and MSI interrupt modes

[PATCH 0/4] fnic: fixup INTx and MSI interrupt modes

[Hannes Reinecke]

Hi all,

with the rework to enable multiqueue the non-multiqueue interrupt modes
(ie MSI and INTx) got broken and resulted in a non-functional drivers.
This patchset fixes up the driver to work in all interrupt modes and
adds a new module parameter 'fnic_intr_mode' to switch between them.

As usual, comments and reviews are welcome.

Hannes Reinecke (4):
  fnic: use resulting interrupt mode during configuration
  fnic: correctly initialize 'fnic->name'
  fnic: missing initialisation for wq_copy_base
  fnic: make interrupt mode configurable

 drivers/scsi/fnic/fnic.h      |  2 +-
 drivers/scsi/fnic/fnic_isr.c  | 15 +++++++++++----
 drivers/scsi/fnic/fnic_main.c | 17 ++++++++++++-----
 3 files changed, 24 insertions(+), 10 deletions(-)

-- 
2.43.0

[PATCH 1/4] fnic: use resulting interrupt mode during configuration

[Hannes Reinecke]

The configured interrupt mode might be different than the resulting
interrupts mode after all configuration limitations have been applied.

Signed-off-by: Hannes Reinecke <hare@kernel.org>
---
 drivers/scsi/fnic/fnic_main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/fnic/fnic_main.c b/drivers/scsi/fnic/fnic_main.c
index 4cc4077ea53c..7075a23d9229 100644
--- a/drivers/scsi/fnic/fnic_main.c
+++ b/drivers/scsi/fnic/fnic_main.c
@@ -678,7 +678,7 @@ void fnic_mq_map_queues_cpus(struct Scsi_Host *host)
 {
 	struct fnic *fnic = *((struct fnic **) shost_priv(host));
 	struct pci_dev *l_pdev = fnic->pdev;
-	int intr_mode = fnic->config.intr_mode;
+	int intr_mode = vnic_dev_get_intr_mode(fnic->vdev);
 	struct blk_mq_queue_map *qmap = &host->tag_set.map[HCTX_TYPE_DEFAULT];
 
 	if (intr_mode == VNIC_DEV_INTR_MODE_MSI || intr_mode == VNIC_DEV_INTR_MODE_INTX) {
-- 
2.43.0

[PATCH 2/4] fnic: correctly initialize 'fnic->name'

[Hannes Reinecke]

The instance name is required for setting up interrupt handlers, so
set it early enough to avoid the request_irq() routine crashing on
duplicate or empty names.

Signed-off-by: Hannes Reinecke <hare@kernel.org>
---
 drivers/scsi/fnic/fnic_main.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/scsi/fnic/fnic_main.c b/drivers/scsi/fnic/fnic_main.c
index 7075a23d9229..870b265be41a 100644
--- a/drivers/scsi/fnic/fnic_main.c
+++ b/drivers/scsi/fnic/fnic_main.c
@@ -575,9 +575,6 @@ static void fnic_scsi_init(struct fnic *fnic)
 {
 	struct Scsi_Host *host = fnic->host;
 
-	snprintf(fnic->name, sizeof(fnic->name) - 1, "%s%d", DRV_NAME,
-			 host->host_no);
-
 	host->transportt = fnic_fc_transport;
 }
 
@@ -732,6 +729,8 @@ static int fnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	fnic->pdev = pdev;
 	fnic->fnic_num = fnic_id;
+	snprintf(fnic->name, sizeof(fnic->name) - 1, "%s%d", DRV_NAME,
+		 fnic->fnic_num);
 
 	/* Find model name from PCIe subsys ID */
 	if (fnic_get_desc_by_devid(pdev, &desc, &subsys_desc) == 0) {
-- 
2.43.0

[PATCH 3/4] fnic: missing initialisation for wq_copy_base

[Hannes Reinecke]

With the conversion to multiqueue the 'wq_copy_base' value were left
uninitialized for MSI and INTx interrupts, causing the driver to issue
a message 'FCPIO_SUCCESS icmnd completion on the wrong queue' and finally
running out of command tags as the completions would be accounted on
the wrong queue.

Fixes: 8a8449ca5e33 ("scsi: fnic: Modify ISRs to support multiqueue (MQ)")

Signed-off-by: Hannes Reinecke <hare@kernel.org>
---
 drivers/scsi/fnic/fnic_isr.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/scsi/fnic/fnic_isr.c b/drivers/scsi/fnic/fnic_isr.c
index 7ed50b11afa6..e16b76d537e8 100644
--- a/drivers/scsi/fnic/fnic_isr.c
+++ b/drivers/scsi/fnic/fnic_isr.c
@@ -350,6 +350,7 @@ int fnic_set_intr_mode(struct fnic *fnic)
 		fnic->cq_count = 3;
 		fnic->intr_count = 1;
 		fnic->err_intr_offset = 0;
+		fnic->copy_wq_base = fnic->rq_count + fnic->raw_wq_count;
 
 		FNIC_ISR_DBG(KERN_DEBUG, fnic->host, fnic->fnic_num,
 			     "Using MSI Interrupts\n");
@@ -376,6 +377,7 @@ int fnic_set_intr_mode(struct fnic *fnic)
 		fnic->wq_copy_count = 1;
 		fnic->cq_count = 3;
 		fnic->intr_count = 3;
+		fnic->copy_wq_base = fnic->rq_count + fnic->raw_wq_count;
 
 		FNIC_ISR_DBG(KERN_DEBUG, fnic->host, fnic->fnic_num,
 			     "Using Legacy Interrupts\n");
-- 
2.43.0

[PATCH 4/4] fnic: make interrupt mode configurable

[Hannes Reinecke]

In some environments (eg kdump) not all CPUs are online, so the MQ
mapping might be resulting in an invalid layout. So make the interrupt
mode settable via an 'fnic_intr_mode' module parameter and switch
to INTx if the 'reset_devices' kernel parameter is specified.

Signed-off-by: Hannes Reinecke <hare@kernel.org>
---
 drivers/scsi/fnic/fnic.h      |  2 +-
 drivers/scsi/fnic/fnic_isr.c  | 13 +++++++++----
 drivers/scsi/fnic/fnic_main.c | 10 +++++++++-
 3 files changed, 19 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/fnic/fnic.h b/drivers/scsi/fnic/fnic.h
index 1199d701c3f5..c679283955e9 100644
--- a/drivers/scsi/fnic/fnic.h
+++ b/drivers/scsi/fnic/fnic.h
@@ -484,7 +484,7 @@ extern struct workqueue_struct *fnic_fip_queue;
 extern const struct attribute_group *fnic_host_groups[];
 
 void fnic_clear_intr_mode(struct fnic *fnic);
-int fnic_set_intr_mode(struct fnic *fnic);
+int fnic_set_intr_mode(struct fnic *fnic, unsigned int mode);
 int fnic_set_intr_mode_msix(struct fnic *fnic);
 void fnic_free_intr(struct fnic *fnic);
 int fnic_request_intr(struct fnic *fnic);
diff --git a/drivers/scsi/fnic/fnic_isr.c b/drivers/scsi/fnic/fnic_isr.c
index e16b76d537e8..b6594ad064ca 100644
--- a/drivers/scsi/fnic/fnic_isr.c
+++ b/drivers/scsi/fnic/fnic_isr.c
@@ -319,20 +319,25 @@ int fnic_set_intr_mode_msix(struct fnic *fnic)
 	return 1;
 }
 
-int fnic_set_intr_mode(struct fnic *fnic)
+int fnic_set_intr_mode(struct fnic *fnic, unsigned int intr_mode)
 {
 	int ret_status = 0;
 
 	/*
 	 * Set interrupt mode (INTx, MSI, MSI-X) depending
 	 * system capabilities.
-	 *
+	 */
+	if (intr_mode != VNIC_DEV_INTR_MODE_MSIX)
+		goto try_msi;
+	/*
 	 * Try MSI-X first
 	 */
 	ret_status = fnic_set_intr_mode_msix(fnic);
 	if (ret_status == 0)
 		return ret_status;
-
+try_msi:
+	if (intr_mode != VNIC_DEV_INTR_MODE_MSI)
+		goto try_intx;
 	/*
 	 * Next try MSI
 	 * We need 1 RQ, 1 WQ, 1 WQ_COPY, 3 CQs, and 1 INTR
@@ -358,7 +363,7 @@ int fnic_set_intr_mode(struct fnic *fnic)
 
 		return 0;
 	}
-
+try_intx:
 	/*
 	 * Next try INTx
 	 * We need 1 RQ, 1 WQ, 1 WQ_COPY, 3 CQs, and 3 INTRs
diff --git a/drivers/scsi/fnic/fnic_main.c b/drivers/scsi/fnic/fnic_main.c
index 870b265be41a..4bdd55958f59 100644
--- a/drivers/scsi/fnic/fnic_main.c
+++ b/drivers/scsi/fnic/fnic_main.c
@@ -97,6 +97,10 @@ module_param(pc_rscn_handling_feature_flag, uint, 0644);
 MODULE_PARM_DESC(pc_rscn_handling_feature_flag,
 		 "PCRSCN handling (0 for none. 1 to handle PCRSCN (default))");
 
+static unsigned int fnic_intr_mode = VNIC_DEV_INTR_MODE_MSIX;
+module_param(fnic_intr_mode, uint, S_IRUGO | S_IWUSR);
+MODULE_PARM_DESC(fnic_intr_mode, "Interrupt mode, 1 = INTx, 2 = MSI, 3 = MSIx (default: 3)");
+
 struct workqueue_struct *reset_fnic_work_queue;
 struct workqueue_struct *fnic_fip_queue;
 
@@ -869,7 +873,11 @@ static int fnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	fnic_get_res_counts(fnic);
 
-	err = fnic_set_intr_mode(fnic);
+	/* Override interrupt selection during kdump */
+	if (reset_devices)
+		fnic_intr_mode = VNIC_DEV_INTR_MODE_INTX;
+
+	err = fnic_set_intr_mode(fnic, fnic_intr_mode);
 	if (err) {
 		dev_err(&fnic->pdev->dev, "Failed to set intr mode, "
 			     "aborting.\n");
-- 
2.43.0

RE: [PATCH 1/4] fnic: use resulting interrupt mode during configuration

[Karan Tilak Kumar (kartilak)]

Cisco Confidential
On Tuesday, November 4, 2025 2:04 AM, Hannes Reinecke <hare@kernel.org> wrote:
>
> The configured interrupt mode might be different than the resulting
> interrupts mode after all configuration limitations have been applied.
>
> Signed-off-by: Hannes Reinecke <hare@kernel.org>
> ---
> drivers/scsi/fnic/fnic_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/fnic/fnic_main.c b/drivers/scsi/fnic/fnic_main.c
> index 4cc4077ea53c..7075a23d9229 100644
> --- a/drivers/scsi/fnic/fnic_main.c
> +++ b/drivers/scsi/fnic/fnic_main.c
> @@ -678,7 +678,7 @@ void fnic_mq_map_queues_cpus(struct Scsi_Host *host)
> {
> struct fnic *fnic = *((struct fnic **) shost_priv(host));
> struct pci_dev *l_pdev = fnic->pdev;
> -     int intr_mode = fnic->config.intr_mode;
> +     int intr_mode = vnic_dev_get_intr_mode(fnic->vdev);
> struct blk_mq_queue_map *qmap = &host->tag_set.map[HCTX_TYPE_DEFAULT];
>
> if (intr_mode == VNIC_DEV_INTR_MODE_MSI || intr_mode == VNIC_DEV_INTR_MODE_INTX) {
> --
> 2.43.0
>
>

Thanks for this change, Hannes.
The fnic team will review this and get back to you.

Regards,
Karan

RE: [PATCH 2/4] fnic: correctly initialize 'fnic->name'

[Karan Tilak Kumar (kartilak)]

Cisco Confidential
On Tuesday, November 4, 2025 2:04 AM, Hannes Reinecke <hare@kernel.org> wrote:
>
> The instance name is required for setting up interrupt handlers, so
> set it early enough to avoid the request_irq() routine crashing on
> duplicate or empty names.
>
> Signed-off-by: Hannes Reinecke <hare@kernel.org>
> ---
> drivers/scsi/fnic/fnic_main.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/fnic/fnic_main.c b/drivers/scsi/fnic/fnic_main.c
> index 7075a23d9229..870b265be41a 100644
> --- a/drivers/scsi/fnic/fnic_main.c
> +++ b/drivers/scsi/fnic/fnic_main.c
> @@ -575,9 +575,6 @@ static void fnic_scsi_init(struct fnic *fnic)
> {
> struct Scsi_Host *host = fnic->host;
>
> -     snprintf(fnic->name, sizeof(fnic->name) - 1, "%s%d", DRV_NAME,
> -                      host->host_no);
> -
> host->transportt = fnic_fc_transport;
> }
>
> @@ -732,6 +729,8 @@ static int fnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>
> fnic->pdev = pdev;
> fnic->fnic_num = fnic_id;
> +     snprintf(fnic->name, sizeof(fnic->name) - 1, "%s%d", DRV_NAME,
> +              fnic->fnic_num);
>
> /* Find model name from PCIe subsys ID */
> if (fnic_get_desc_by_devid(pdev, &desc, &subsys_desc) == 0) {
> --
> 2.43.0
>
>

Thanks for this change, Hannes.
The fnic team will review this and get back to you.

Regards,
Karan

RE: [PATCH 3/4] fnic: missing initialisation for wq_copy_base

[Karan Tilak Kumar (kartilak)]

Cisco Confidential
On Tuesday, November 4, 2025 2:04 AM, Hannes Reinecke <hare@kernel.org> wrote:
>
> With the conversion to multiqueue the 'wq_copy_base' value were left
> uninitialized for MSI and INTx interrupts, causing the driver to issue
> a message 'FCPIO_SUCCESS icmnd completion on the wrong queue' and finally
> running out of command tags as the completions would be accounted on
> the wrong queue.
>
> Fixes: 8a8449ca5e33 ("scsi: fnic: Modify ISRs to support multiqueue (MQ)")
>
> Signed-off-by: Hannes Reinecke <hare@kernel.org>
> ---
> drivers/scsi/fnic/fnic_isr.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/scsi/fnic/fnic_isr.c b/drivers/scsi/fnic/fnic_isr.c
> index 7ed50b11afa6..e16b76d537e8 100644
> --- a/drivers/scsi/fnic/fnic_isr.c
> +++ b/drivers/scsi/fnic/fnic_isr.c
> @@ -350,6 +350,7 @@ int fnic_set_intr_mode(struct fnic *fnic)
> fnic->cq_count = 3;
> fnic->intr_count = 1;
> fnic->err_intr_offset = 0;
> +             fnic->copy_wq_base = fnic->rq_count + fnic->raw_wq_count;
>
> FNIC_ISR_DBG(KERN_DEBUG, fnic->host, fnic->fnic_num,
> "Using MSI Interrupts\n");
> @@ -376,6 +377,7 @@ int fnic_set_intr_mode(struct fnic *fnic)
> fnic->wq_copy_count = 1;
> fnic->cq_count = 3;
> fnic->intr_count = 3;
> +             fnic->copy_wq_base = fnic->rq_count + fnic->raw_wq_count;
>
> FNIC_ISR_DBG(KERN_DEBUG, fnic->host, fnic->fnic_num,
> "Using Legacy Interrupts\n");
> --
> 2.43.0
>
>

Thanks for this change, Hannes.
The fnic team will review this and get back to you.

Regards,
Karan

RE: [PATCH 4/4] fnic: make interrupt mode configurable

[Karan Tilak Kumar (kartilak)]

Cisco Confidential
On Tuesday, November 4, 2025 2:04 AM, Hannes Reinecke <hare@kernel.org> wrote:
>
> In some environments (eg kdump) not all CPUs are online, so the MQ
> mapping might be resulting in an invalid layout. So make the interrupt
> mode settable via an 'fnic_intr_mode' module parameter and switch
> to INTx if the 'reset_devices' kernel parameter is specified.
>
> Signed-off-by: Hannes Reinecke <hare@kernel.org>
> ---
> drivers/scsi/fnic/fnic.h      |  2 +-
> drivers/scsi/fnic/fnic_isr.c  | 13 +++++++++----
> drivers/scsi/fnic/fnic_main.c | 10 +++++++++-
> 3 files changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/scsi/fnic/fnic.h b/drivers/scsi/fnic/fnic.h
> index 1199d701c3f5..c679283955e9 100644
> --- a/drivers/scsi/fnic/fnic.h
> +++ b/drivers/scsi/fnic/fnic.h
> @@ -484,7 +484,7 @@ extern struct workqueue_struct *fnic_fip_queue;
> extern const struct attribute_group *fnic_host_groups[];
>
> void fnic_clear_intr_mode(struct fnic *fnic);
> -int fnic_set_intr_mode(struct fnic *fnic);
> +int fnic_set_intr_mode(struct fnic *fnic, unsigned int mode);
> int fnic_set_intr_mode_msix(struct fnic *fnic);
> void fnic_free_intr(struct fnic *fnic);
> int fnic_request_intr(struct fnic *fnic);
> diff --git a/drivers/scsi/fnic/fnic_isr.c b/drivers/scsi/fnic/fnic_isr.c
> index e16b76d537e8..b6594ad064ca 100644
> --- a/drivers/scsi/fnic/fnic_isr.c
> +++ b/drivers/scsi/fnic/fnic_isr.c
> @@ -319,20 +319,25 @@ int fnic_set_intr_mode_msix(struct fnic *fnic)
> return 1;
> }
>
> -int fnic_set_intr_mode(struct fnic *fnic)
> +int fnic_set_intr_mode(struct fnic *fnic, unsigned int intr_mode)
> {
> int ret_status = 0;
>
> /*
> * Set interrupt mode (INTx, MSI, MSI-X) depending
> * system capabilities.
> -      *
> +      */
> +     if (intr_mode != VNIC_DEV_INTR_MODE_MSIX)
> +             goto try_msi;
> +     /*
> * Try MSI-X first
> */
> ret_status = fnic_set_intr_mode_msix(fnic);
> if (ret_status == 0)
> return ret_status;
> -
> +try_msi:
> +     if (intr_mode != VNIC_DEV_INTR_MODE_MSI)
> +             goto try_intx;
> /*
> * Next try MSI
> * We need 1 RQ, 1 WQ, 1 WQ_COPY, 3 CQs, and 1 INTR
> @@ -358,7 +363,7 @@ int fnic_set_intr_mode(struct fnic *fnic)
>
> return 0;
> }
> -
> +try_intx:
> /*
> * Next try INTx
> * We need 1 RQ, 1 WQ, 1 WQ_COPY, 3 CQs, and 3 INTRs
> diff --git a/drivers/scsi/fnic/fnic_main.c b/drivers/scsi/fnic/fnic_main.c
> index 870b265be41a..4bdd55958f59 100644
> --- a/drivers/scsi/fnic/fnic_main.c
> +++ b/drivers/scsi/fnic/fnic_main.c
> @@ -97,6 +97,10 @@ module_param(pc_rscn_handling_feature_flag, uint, 0644);
> MODULE_PARM_DESC(pc_rscn_handling_feature_flag,
> "PCRSCN handling (0 for none. 1 to handle PCRSCN (default))");
>
> +static unsigned int fnic_intr_mode = VNIC_DEV_INTR_MODE_MSIX;
> +module_param(fnic_intr_mode, uint, S_IRUGO | S_IWUSR);
> +MODULE_PARM_DESC(fnic_intr_mode, "Interrupt mode, 1 = INTx, 2 = MSI, 3 = MSIx (default: 3)");
> +
> struct workqueue_struct *reset_fnic_work_queue;
> struct workqueue_struct *fnic_fip_queue;
>
> @@ -869,7 +873,11 @@ static int fnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>
> fnic_get_res_counts(fnic);
>
> -     err = fnic_set_intr_mode(fnic);
> +     /* Override interrupt selection during kdump */
> +     if (reset_devices)
> +             fnic_intr_mode = VNIC_DEV_INTR_MODE_INTX;
> +
> +     err = fnic_set_intr_mode(fnic, fnic_intr_mode);
> if (err) {
> dev_err(&fnic->pdev->dev, "Failed to set intr mode, "
> "aborting.\n");
> --
> 2.43.0
>
>

Thanks for this change, Hannes.
The fnic team will review this and get back to you.

Regards,
Karan

RE: [PATCH 1/4] fnic: use resulting interrupt mode during configuration

[Karan Tilak Kumar (kartilak)]

Cisco Confidential
On Tuesday, November 4, 2025 2:04 AM, Hannes Reinecke <hare@kernel.org> wrote:
>
> The configured interrupt mode might be different than the resulting
> interrupts mode after all configuration limitations have been applied.
>
> Signed-off-by: Hannes Reinecke <hare@kernel.org>
> ---
> drivers/scsi/fnic/fnic_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/fnic/fnic_main.c b/drivers/scsi/fnic/fnic_main.c
> index 4cc4077ea53c..7075a23d9229 100644
> --- a/drivers/scsi/fnic/fnic_main.c
> +++ b/drivers/scsi/fnic/fnic_main.c
> @@ -678,7 +678,7 @@ void fnic_mq_map_queues_cpus(struct Scsi_Host *host)
> {
> struct fnic *fnic = *((struct fnic **) shost_priv(host));
> struct pci_dev *l_pdev = fnic->pdev;
> -     int intr_mode = fnic->config.intr_mode;
> +     int intr_mode = vnic_dev_get_intr_mode(fnic->vdev);
> struct blk_mq_queue_map *qmap = &host->tag_set.map[HCTX_TYPE_DEFAULT];
>
> if (intr_mode == VNIC_DEV_INTR_MODE_MSI || intr_mode == VNIC_DEV_INTR_MODE_INTX) {
> --
> 2.43.0
>
>

Thanks for this change, Hannes. It looks good.

Reviewed-by: Karan Tilak Kumar <kartilak@cisco.com>

Regards,
Karan

RE: [PATCH 2/4] fnic: correctly initialize 'fnic->name'

[Karan Tilak Kumar (kartilak)]

Cisco Confidential
On Tuesday, November 4, 2025 2:04 AM, Hannes Reinecke <hare@kernel.org> wrote:
>
> The instance name is required for setting up interrupt handlers, so
> set it early enough to avoid the request_irq() routine crashing on
> duplicate or empty names.
>
> Signed-off-by: Hannes Reinecke <hare@kernel.org>
> ---
> drivers/scsi/fnic/fnic_main.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/scsi/fnic/fnic_main.c b/drivers/scsi/fnic/fnic_main.c
> index 7075a23d9229..870b265be41a 100644
> --- a/drivers/scsi/fnic/fnic_main.c
> +++ b/drivers/scsi/fnic/fnic_main.c
> @@ -575,9 +575,6 @@ static void fnic_scsi_init(struct fnic *fnic)
> {
> struct Scsi_Host *host = fnic->host;
>
> -     snprintf(fnic->name, sizeof(fnic->name) - 1, "%s%d", DRV_NAME,
> -                      host->host_no);
> -
> host->transportt = fnic_fc_transport;
> }
>
> @@ -732,6 +729,8 @@ static int fnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>
> fnic->pdev = pdev;
> fnic->fnic_num = fnic_id;
> +     snprintf(fnic->name, sizeof(fnic->name) - 1, "%s%d", DRV_NAME,
> +              fnic->fnic_num);
>
> /* Find model name from PCIe subsys ID */
> if (fnic_get_desc_by_devid(pdev, &desc, &subsys_desc) == 0) {
> --
> 2.43.0
>
>

Thanks for this change, Hannes. It looks good.

Reviewed-by: Karan Tilak Kumar <kartilak@cisco.com>

Regards,
Karan

RE: [PATCH 3/4] fnic: missing initialisation for wq_copy_base

[Karan Tilak Kumar (kartilak)]

Cisco Confidential
On Tuesday, November 4, 2025 2:04 AM, Hannes Reinecke <hare@kernel.org> wrote:
>
> With the conversion to multiqueue the 'wq_copy_base' value were left
> uninitialized for MSI and INTx interrupts, causing the driver to issue
> a message 'FCPIO_SUCCESS icmnd completion on the wrong queue' and finally
> running out of command tags as the completions would be accounted on
> the wrong queue.
>
> Fixes: 8a8449ca5e33 ("scsi: fnic: Modify ISRs to support multiqueue (MQ)")
>
> Signed-off-by: Hannes Reinecke <hare@kernel.org>
> ---
> drivers/scsi/fnic/fnic_isr.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/scsi/fnic/fnic_isr.c b/drivers/scsi/fnic/fnic_isr.c
> index 7ed50b11afa6..e16b76d537e8 100644
> --- a/drivers/scsi/fnic/fnic_isr.c
> +++ b/drivers/scsi/fnic/fnic_isr.c
> @@ -350,6 +350,7 @@ int fnic_set_intr_mode(struct fnic *fnic)
> fnic->cq_count = 3;
> fnic->intr_count = 1;
> fnic->err_intr_offset = 0;
> +             fnic->copy_wq_base = fnic->rq_count + fnic->raw_wq_count;
>
> FNIC_ISR_DBG(KERN_DEBUG, fnic->host, fnic->fnic_num,
> "Using MSI Interrupts\n");
> @@ -376,6 +377,7 @@ int fnic_set_intr_mode(struct fnic *fnic)
> fnic->wq_copy_count = 1;
> fnic->cq_count = 3;
> fnic->intr_count = 3;
> +             fnic->copy_wq_base = fnic->rq_count + fnic->raw_wq_count;
>
> FNIC_ISR_DBG(KERN_DEBUG, fnic->host, fnic->fnic_num,
> "Using Legacy Interrupts\n");
> --
> 2.43.0
>
>

Thanks for these changes, Hannes.

I will need to test these changes and get back to you.

Regards,
Karan

RE: [PATCH 4/4] fnic: make interrupt mode configurable

[Karan Tilak Kumar (kartilak)]

Cisco Confidential
On Tuesday, November 4, 2025 2:04 AM, Hannes Reinecke <hare@kernel.org> wrote:
>
> In some environments (eg kdump) not all CPUs are online, so the MQ
> mapping might be resulting in an invalid layout. So make the interrupt
> mode settable via an 'fnic_intr_mode' module parameter and switch
> to INTx if the 'reset_devices' kernel parameter is specified.
>
> Signed-off-by: Hannes Reinecke <hare@kernel.org>
> ---
> drivers/scsi/fnic/fnic.h      |  2 +-
> drivers/scsi/fnic/fnic_isr.c  | 13 +++++++++----
> drivers/scsi/fnic/fnic_main.c | 10 +++++++++-
> 3 files changed, 19 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/scsi/fnic/fnic.h b/drivers/scsi/fnic/fnic.h
> index 1199d701c3f5..c679283955e9 100644
> --- a/drivers/scsi/fnic/fnic.h
> +++ b/drivers/scsi/fnic/fnic.h
> @@ -484,7 +484,7 @@ extern struct workqueue_struct *fnic_fip_queue;
> extern const struct attribute_group *fnic_host_groups[];
>
> void fnic_clear_intr_mode(struct fnic *fnic);
> -int fnic_set_intr_mode(struct fnic *fnic);
> +int fnic_set_intr_mode(struct fnic *fnic, unsigned int mode);
> int fnic_set_intr_mode_msix(struct fnic *fnic);
> void fnic_free_intr(struct fnic *fnic);
> int fnic_request_intr(struct fnic *fnic);
> diff --git a/drivers/scsi/fnic/fnic_isr.c b/drivers/scsi/fnic/fnic_isr.c
> index e16b76d537e8..b6594ad064ca 100644
> --- a/drivers/scsi/fnic/fnic_isr.c
> +++ b/drivers/scsi/fnic/fnic_isr.c
> @@ -319,20 +319,25 @@ int fnic_set_intr_mode_msix(struct fnic *fnic)
> return 1;
> }
>
> -int fnic_set_intr_mode(struct fnic *fnic)
> +int fnic_set_intr_mode(struct fnic *fnic, unsigned int intr_mode)
> {
> int ret_status = 0;
>
> /*
> * Set interrupt mode (INTx, MSI, MSI-X) depending
> * system capabilities.
> -      *
> +      */
> +     if (intr_mode != VNIC_DEV_INTR_MODE_MSIX)
> +             goto try_msi;
> +     /*
> * Try MSI-X first
> */
> ret_status = fnic_set_intr_mode_msix(fnic);
> if (ret_status == 0)
> return ret_status;
> -
> +try_msi:
> +     if (intr_mode != VNIC_DEV_INTR_MODE_MSI)
> +             goto try_intx;
> /*
> * Next try MSI
> * We need 1 RQ, 1 WQ, 1 WQ_COPY, 3 CQs, and 1 INTR
> @@ -358,7 +363,7 @@ int fnic_set_intr_mode(struct fnic *fnic)
>
> return 0;
> }
> -
> +try_intx:
> /*
> * Next try INTx
> * We need 1 RQ, 1 WQ, 1 WQ_COPY, 3 CQs, and 3 INTRs
> diff --git a/drivers/scsi/fnic/fnic_main.c b/drivers/scsi/fnic/fnic_main.c
> index 870b265be41a..4bdd55958f59 100644
> --- a/drivers/scsi/fnic/fnic_main.c
> +++ b/drivers/scsi/fnic/fnic_main.c
> @@ -97,6 +97,10 @@ module_param(pc_rscn_handling_feature_flag, uint, 0644);
> MODULE_PARM_DESC(pc_rscn_handling_feature_flag,
> "PCRSCN handling (0 for none. 1 to handle PCRSCN (default))");
>
> +static unsigned int fnic_intr_mode = VNIC_DEV_INTR_MODE_MSIX;
> +module_param(fnic_intr_mode, uint, S_IRUGO | S_IWUSR);
> +MODULE_PARM_DESC(fnic_intr_mode, "Interrupt mode, 1 = INTx, 2 = MSI, 3 = MSIx (default: 3)");

Based on fnic team's review: there is a way to choose the interrupt mode using the UCS management platform.
We do not want to expose this as a module parameter.

> struct workqueue_struct *reset_fnic_work_queue;
> struct workqueue_struct *fnic_fip_queue;
>
> @@ -869,7 +873,11 @@ static int fnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>
> fnic_get_res_counts(fnic);
>
> -     err = fnic_set_intr_mode(fnic);
> +     /* Override interrupt selection during kdump */
> +     if (reset_devices)
> +             fnic_intr_mode = VNIC_DEV_INTR_MODE_INTX;
> +
> +     err = fnic_set_intr_mode(fnic, fnic_intr_mode);
> if (err) {
> dev_err(&fnic->pdev->dev, "Failed to set intr mode, "
> "aborting.\n");
> --
> 2.43.0
>
>

Thanks for these changes, Hannes.
For the other changes in this patch, I will need to test and get back to you.

Regards,
Karan

Re: [PATCH 4/4] fnic: make interrupt mode configurable

[Hannes Reinecke]

On 11/6/25 23:09, Karan Tilak Kumar (kartilak) wrote:
> 
> Cisco Confidential
> On Tuesday, November 4, 2025 2:04 AM, Hannes Reinecke <hare@kernel.org> wrote:
>>
>> In some environments (eg kdump) not all CPUs are online, so the MQ
>> mapping might be resulting in an invalid layout. So make the interrupt
>> mode settable via an 'fnic_intr_mode' module parameter and switch
>> to INTx if the 'reset_devices' kernel parameter is specified.
>>
>> Signed-off-by: Hannes Reinecke <hare@kernel.org>
>> ---
>> drivers/scsi/fnic/fnic.h      |  2 +-
>> drivers/scsi/fnic/fnic_isr.c  | 13 +++++++++----
>> drivers/scsi/fnic/fnic_main.c | 10 +++++++++-
>> 3 files changed, 19 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/scsi/fnic/fnic.h b/drivers/scsi/fnic/fnic.h
>> index 1199d701c3f5..c679283955e9 100644
>> --- a/drivers/scsi/fnic/fnic.h
>> +++ b/drivers/scsi/fnic/fnic.h
>> @@ -484,7 +484,7 @@ extern struct workqueue_struct *fnic_fip_queue;
>> extern const struct attribute_group *fnic_host_groups[];
>>
>> void fnic_clear_intr_mode(struct fnic *fnic);
>> -int fnic_set_intr_mode(struct fnic *fnic);
>> +int fnic_set_intr_mode(struct fnic *fnic, unsigned int mode);
>> int fnic_set_intr_mode_msix(struct fnic *fnic);
>> void fnic_free_intr(struct fnic *fnic);
>> int fnic_request_intr(struct fnic *fnic);
>> diff --git a/drivers/scsi/fnic/fnic_isr.c b/drivers/scsi/fnic/fnic_isr.c
>> index e16b76d537e8..b6594ad064ca 100644
>> --- a/drivers/scsi/fnic/fnic_isr.c
>> +++ b/drivers/scsi/fnic/fnic_isr.c
>> @@ -319,20 +319,25 @@ int fnic_set_intr_mode_msix(struct fnic *fnic)
>> return 1;
>> }
>>
>> -int fnic_set_intr_mode(struct fnic *fnic)
>> +int fnic_set_intr_mode(struct fnic *fnic, unsigned int intr_mode)
>> {
>> int ret_status = 0;
>>
>> /*
>> * Set interrupt mode (INTx, MSI, MSI-X) depending
>> * system capabilities.
>> -      *
>> +      */
>> +     if (intr_mode != VNIC_DEV_INTR_MODE_MSIX)
>> +             goto try_msi;
>> +     /*
>> * Try MSI-X first
>> */
>> ret_status = fnic_set_intr_mode_msix(fnic);
>> if (ret_status == 0)
>> return ret_status;
>> -
>> +try_msi:
>> +     if (intr_mode != VNIC_DEV_INTR_MODE_MSI)
>> +             goto try_intx;
>> /*
>> * Next try MSI
>> * We need 1 RQ, 1 WQ, 1 WQ_COPY, 3 CQs, and 1 INTR
>> @@ -358,7 +363,7 @@ int fnic_set_intr_mode(struct fnic *fnic)
>>
>> return 0;
>> }
>> -
>> +try_intx:
>> /*
>> * Next try INTx
>> * We need 1 RQ, 1 WQ, 1 WQ_COPY, 3 CQs, and 3 INTRs
>> diff --git a/drivers/scsi/fnic/fnic_main.c b/drivers/scsi/fnic/fnic_main.c
>> index 870b265be41a..4bdd55958f59 100644
>> --- a/drivers/scsi/fnic/fnic_main.c
>> +++ b/drivers/scsi/fnic/fnic_main.c
>> @@ -97,6 +97,10 @@ module_param(pc_rscn_handling_feature_flag, uint, 0644);
>> MODULE_PARM_DESC(pc_rscn_handling_feature_flag,
>> "PCRSCN handling (0 for none. 1 to handle PCRSCN (default))");
>>
>> +static unsigned int fnic_intr_mode = VNIC_DEV_INTR_MODE_MSIX;
>> +module_param(fnic_intr_mode, uint, S_IRUGO | S_IWUSR);
>> +MODULE_PARM_DESC(fnic_intr_mode, "Interrupt mode, 1 = INTx, 2 = MSI, 3 = MSIx (default: 3)");
> 
> Based on fnic team's review: there is a way to choose the interrupt mode using the UCS management platform.
> We do not want to expose this as a module parameter.
> 
Yeah, I know. It was primarily used during testing to easily change 
between the various modes.
I can drop it for the next round.

>> struct workqueue_struct *reset_fnic_work_queue;
>> struct workqueue_struct *fnic_fip_queue;
>>
>> @@ -869,7 +873,11 @@ static int fnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>>
>> fnic_get_res_counts(fnic);
>>
>> -     err = fnic_set_intr_mode(fnic);
>> +     /* Override interrupt selection during kdump */
>> +     if (reset_devices)
>> +             fnic_intr_mode = VNIC_DEV_INTR_MODE_INTX;
>> +
>> +     err = fnic_set_intr_mode(fnic, fnic_intr_mode);
>> if (err) {
>> dev_err(&fnic->pdev->dev, "Failed to set intr mode, "
>> "aborting.\n");
>> --
>> 2.43.0
>>
>>
> 
> Thanks for these changes, Hannes.
> For the other changes in this patch, I will need to test and get back to you.
> 

Thanks.
The 'reset_devices' thing is primarily there for kdump; might be an idea 
to make is explicit by using 'is_kdump_kernel()' directly.

Cheers,

Hannes

-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

RE: [PATCH 4/4] fnic: make interrupt mode configurable

[Karan Tilak Kumar (kartilak)]

Cisco Confidential
On Friday, November 7, 2025 12:29 AM, Hannes Reinecke <hare@suse.de> wrote:
>
> On 11/6/25 23:09, Karan Tilak Kumar (kartilak) wrote:
> >
> > Cisco Confidential
> > On Tuesday, November 4, 2025 2:04 AM, Hannes Reinecke <hare@kernel.org> wrote:
> >>
> >> In some environments (eg kdump) not all CPUs are online, so the MQ
> >> mapping might be resulting in an invalid layout. So make the interrupt
> >> mode settable via an 'fnic_intr_mode' module parameter and switch
> >> to INTx if the 'reset_devices' kernel parameter is specified.
> >>
> >> Signed-off-by: Hannes Reinecke <hare@kernel.org>
> >> ---
> >> drivers/scsi/fnic/fnic.h      |  2 +-
> >> drivers/scsi/fnic/fnic_isr.c  | 13 +++++++++----
> >> drivers/scsi/fnic/fnic_main.c | 10 +++++++++-
> >> 3 files changed, 19 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/drivers/scsi/fnic/fnic.h b/drivers/scsi/fnic/fnic.h
> >> index 1199d701c3f5..c679283955e9 100644
> >> --- a/drivers/scsi/fnic/fnic.h
> >> +++ b/drivers/scsi/fnic/fnic.h
> >> @@ -484,7 +484,7 @@ extern struct workqueue_struct *fnic_fip_queue;
> >> extern const struct attribute_group *fnic_host_groups[];
> >>
> >> void fnic_clear_intr_mode(struct fnic *fnic);
> >> -int fnic_set_intr_mode(struct fnic *fnic);
> >> +int fnic_set_intr_mode(struct fnic *fnic, unsigned int mode);
> >> int fnic_set_intr_mode_msix(struct fnic *fnic);
> >> void fnic_free_intr(struct fnic *fnic);
> >> int fnic_request_intr(struct fnic *fnic);
> >> diff --git a/drivers/scsi/fnic/fnic_isr.c b/drivers/scsi/fnic/fnic_isr.c
> >> index e16b76d537e8..b6594ad064ca 100644
> >> --- a/drivers/scsi/fnic/fnic_isr.c
> >> +++ b/drivers/scsi/fnic/fnic_isr.c
> >> @@ -319,20 +319,25 @@ int fnic_set_intr_mode_msix(struct fnic *fnic)
> >> return 1;
> >> }
> >>
> >> -int fnic_set_intr_mode(struct fnic *fnic)
> >> +int fnic_set_intr_mode(struct fnic *fnic, unsigned int intr_mode)
> >> {
> >> int ret_status = 0;
> >>
> >> /*
> >> * Set interrupt mode (INTx, MSI, MSI-X) depending
> >> * system capabilities.
> >> -      *
> >> +      */
> >> +     if (intr_mode != VNIC_DEV_INTR_MODE_MSIX)
> >> +             goto try_msi;
> >> +     /*
> >> * Try MSI-X first
> >> */
> >> ret_status = fnic_set_intr_mode_msix(fnic);
> >> if (ret_status == 0)
> >> return ret_status;
> >> -
> >> +try_msi:
> >> +     if (intr_mode != VNIC_DEV_INTR_MODE_MSI)
> >> +             goto try_intx;
> >> /*
> >> * Next try MSI
> >> * We need 1 RQ, 1 WQ, 1 WQ_COPY, 3 CQs, and 1 INTR
> >> @@ -358,7 +363,7 @@ int fnic_set_intr_mode(struct fnic *fnic)
> >>
> >> return 0;
> >> }
> >> -
> >> +try_intx:
> >> /*
> >> * Next try INTx
> >> * We need 1 RQ, 1 WQ, 1 WQ_COPY, 3 CQs, and 3 INTRs
> >> diff --git a/drivers/scsi/fnic/fnic_main.c b/drivers/scsi/fnic/fnic_main.c
> >> index 870b265be41a..4bdd55958f59 100644
> >> --- a/drivers/scsi/fnic/fnic_main.c
> >> +++ b/drivers/scsi/fnic/fnic_main.c
> >> @@ -97,6 +97,10 @@ module_param(pc_rscn_handling_feature_flag, uint, 0644);
> >> MODULE_PARM_DESC(pc_rscn_handling_feature_flag,
> >> "PCRSCN handling (0 for none. 1 to handle PCRSCN (default))");
> >>
> >> +static unsigned int fnic_intr_mode = VNIC_DEV_INTR_MODE_MSIX;
> >> +module_param(fnic_intr_mode, uint, S_IRUGO | S_IWUSR);
> >> +MODULE_PARM_DESC(fnic_intr_mode, "Interrupt mode, 1 = INTx, 2 = MSI, 3 = MSIx (default: 3)");
> >
> > Based on fnic team's review: there is a way to choose the interrupt mode using the UCS management platform.
> > We do not want to expose this as a module parameter.
> >
> Yeah, I know. It was primarily used during testing to easily change
> between the various modes.
> I can drop it for the next round.

Thanks Hannes. Sounds good.

> >> struct workqueue_struct *reset_fnic_work_queue;
> >> struct workqueue_struct *fnic_fip_queue;
> >>
> >> @@ -869,7 +873,11 @@ static int fnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >>
> >> fnic_get_res_counts(fnic);
> >>
> >> -     err = fnic_set_intr_mode(fnic);
> >> +     /* Override interrupt selection during kdump */
> >> +     if (reset_devices)
> >> +             fnic_intr_mode = VNIC_DEV_INTR_MODE_INTX;
> >> +
> >> +     err = fnic_set_intr_mode(fnic, fnic_intr_mode);
> >> if (err) {
> >> dev_err(&fnic->pdev->dev, "Failed to set intr mode, "
> >> "aborting.\n");
> >> --
> >> 2.43.0
> >>
> >>
> >
> > Thanks for these changes, Hannes.
> > For the other changes in this patch, I will need to test and get back to you.
> >
>
> Thanks.
> The 'reset_devices' thing is primarily there for kdump; might be an idea
> to make is explicit by using 'is_kdump_kernel()' directly.

Yes Hannes. That would be better. Thanks.

> Cheers,
>
> Hannes
>
> --
> Dr. Hannes Reinecke                  Kernel Storage Architect
> hare@suse.de                                +49 911 74053 688
> SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
> HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
>

I'm bringing up a setup to test. I'll wait for your next revision.

My plan is to induce a kdump to test out these changes.
If you have any other tests in mind, please let me know.

Thanks,
Karan

RE: [PATCH 4/4] fnic: make interrupt mode configurable

[Karan Tilak Kumar (kartilak)]

Cisco Confidential
On Friday, November 7, 2025 2:23 PM, Karan Tilak Kumar (kartilak) wrote:
>
> On Friday, November 7, 2025 12:29 AM, Hannes Reinecke <hare@suse.de> wrote:
> >
> > On 11/6/25 23:09, Karan Tilak Kumar (kartilak) wrote:
> > >
> > > Cisco Confidential
> > > On Tuesday, November 4, 2025 2:04 AM, Hannes Reinecke <hare@kernel.org> wrote:
> > >>
> > >> In some environments (eg kdump) not all CPUs are online, so the MQ
> > >> mapping might be resulting in an invalid layout. So make the interrupt
> > >> mode settable via an 'fnic_intr_mode' module parameter and switch
> > >> to INTx if the 'reset_devices' kernel parameter is specified.
> > >>
> > >> Signed-off-by: Hannes Reinecke <hare@kernel.org>
> > >> ---
> > >> drivers/scsi/fnic/fnic.h      |  2 +-
> > >> drivers/scsi/fnic/fnic_isr.c  | 13 +++++++++----
> > >> drivers/scsi/fnic/fnic_main.c | 10 +++++++++-
> > >> 3 files changed, 19 insertions(+), 6 deletions(-)
> > >>
> > >> diff --git a/drivers/scsi/fnic/fnic.h b/drivers/scsi/fnic/fnic.h
> > >> index 1199d701c3f5..c679283955e9 100644
> > >> --- a/drivers/scsi/fnic/fnic.h
> > >> +++ b/drivers/scsi/fnic/fnic.h
> > >> @@ -484,7 +484,7 @@ extern struct workqueue_struct *fnic_fip_queue;
> > >> extern const struct attribute_group *fnic_host_groups[];
> > >>
> > >> void fnic_clear_intr_mode(struct fnic *fnic);
> > >> -int fnic_set_intr_mode(struct fnic *fnic);
> > >> +int fnic_set_intr_mode(struct fnic *fnic, unsigned int mode);
> > >> int fnic_set_intr_mode_msix(struct fnic *fnic);
> > >> void fnic_free_intr(struct fnic *fnic);
> > >> int fnic_request_intr(struct fnic *fnic);
> > >> diff --git a/drivers/scsi/fnic/fnic_isr.c b/drivers/scsi/fnic/fnic_isr.c
> > >> index e16b76d537e8..b6594ad064ca 100644
> > >> --- a/drivers/scsi/fnic/fnic_isr.c
> > >> +++ b/drivers/scsi/fnic/fnic_isr.c
> > >> @@ -319,20 +319,25 @@ int fnic_set_intr_mode_msix(struct fnic *fnic)
> > >> return 1;
> > >> }
> > >>
> > >> -int fnic_set_intr_mode(struct fnic *fnic)
> > >> +int fnic_set_intr_mode(struct fnic *fnic, unsigned int intr_mode)
> > >> {
> > >> int ret_status = 0;
> > >>
> > >> /*
> > >> * Set interrupt mode (INTx, MSI, MSI-X) depending
> > >> * system capabilities.
> > >> -      *
> > >> +      */
> > >> +     if (intr_mode != VNIC_DEV_INTR_MODE_MSIX)
> > >> +             goto try_msi;
> > >> +     /*
> > >> * Try MSI-X first
> > >> */
> > >> ret_status = fnic_set_intr_mode_msix(fnic);
> > >> if (ret_status == 0)
> > >> return ret_status;
> > >> -
> > >> +try_msi:
> > >> +     if (intr_mode != VNIC_DEV_INTR_MODE_MSI)
> > >> +             goto try_intx;
> > >> /*
> > >> * Next try MSI
> > >> * We need 1 RQ, 1 WQ, 1 WQ_COPY, 3 CQs, and 1 INTR
> > >> @@ -358,7 +363,7 @@ int fnic_set_intr_mode(struct fnic *fnic)
> > >>
> > >> return 0;
> > >> }
> > >> -
> > >> +try_intx:
> > >> /*
> > >> * Next try INTx
> > >> * We need 1 RQ, 1 WQ, 1 WQ_COPY, 3 CQs, and 3 INTRs
> > >> diff --git a/drivers/scsi/fnic/fnic_main.c b/drivers/scsi/fnic/fnic_main.c
> > >> index 870b265be41a..4bdd55958f59 100644
> > >> --- a/drivers/scsi/fnic/fnic_main.c
> > >> +++ b/drivers/scsi/fnic/fnic_main.c
> > >> @@ -97,6 +97,10 @@ module_param(pc_rscn_handling_feature_flag, uint, 0644);
> > >> MODULE_PARM_DESC(pc_rscn_handling_feature_flag,
> > >> "PCRSCN handling (0 for none. 1 to handle PCRSCN (default))");
> > >>
> > >> +static unsigned int fnic_intr_mode = VNIC_DEV_INTR_MODE_MSIX;
> > >> +module_param(fnic_intr_mode, uint, S_IRUGO | S_IWUSR);
> > >> +MODULE_PARM_DESC(fnic_intr_mode, "Interrupt mode, 1 = INTx, 2 = MSI, 3 = MSIx (default: 3)");
> > >
> > > Based on fnic team's review: there is a way to choose the interrupt mode using the UCS management platform.
> > > We do not want to expose this as a module parameter.
> > >
> > Yeah, I know. It was primarily used during testing to easily change
> > between the various modes.
> > I can drop it for the next round.
>
> Thanks Hannes. Sounds good.
>
> > >> struct workqueue_struct *reset_fnic_work_queue;
> > >> struct workqueue_struct *fnic_fip_queue;
> > >>
> > >> @@ -869,7 +873,11 @@ static int fnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > >>
> > >> fnic_get_res_counts(fnic);
> > >>
> > >> -     err = fnic_set_intr_mode(fnic);
> > >> +     /* Override interrupt selection during kdump */
> > >> +     if (reset_devices)
> > >> +             fnic_intr_mode = VNIC_DEV_INTR_MODE_INTX;
> > >> +
> > >> +     err = fnic_set_intr_mode(fnic, fnic_intr_mode);
> > >> if (err) {
> > >> dev_err(&fnic->pdev->dev, "Failed to set intr mode, "
> > >> "aborting.\n");
> > >> --
> > >> 2.43.0
> > >>
> > >>
> > >
> > > Thanks for these changes, Hannes.
> > > For the other changes in this patch, I will need to test and get back to you.
> > >
> >
> > Thanks.
> > The 'reset_devices' thing is primarily there for kdump; might be an idea
> > to make is explicit by using 'is_kdump_kernel()' directly.
>
> Yes Hannes. That would be better. Thanks.
>
> > Cheers,
> >
> > Hannes
> >
> > --
> > Dr. Hannes Reinecke                  Kernel Storage Architect
> > hare@suse.de                                +49 911 74053 688
> > SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
> > HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
> >
>
> I'm bringing up a setup to test. I'll wait for your next revision.
>
> My plan is to induce a kdump to test out these changes.
> If you have any other tests in mind, please let me know.
>
> Thanks,
> Karan
>

Hi Hannes,

Please consider sending out a new revision of these changes.
I'd like to test all the changes and add a tested-by tag to them.

Thanks,
Karan

Re: [PATCH 4/4] fnic: make interrupt mode configurable

[Lee Duncan]

On Wed, Jan 14, 2026 at 9:04 AM Karan Tilak Kumar (kartilak)
<kartilak@cisco.com> wrote:
>
>
> Cisco Confidential
> On Friday, November 7, 2025 2:23 PM, Karan Tilak Kumar (kartilak) wrote:
> >
> > On Friday, November 7, 2025 12:29 AM, Hannes Reinecke <hare@suse.de> wrote:
> > >
> > > On 11/6/25 23:09, Karan Tilak Kumar (kartilak) wrote:
> > > >
> > > > Cisco Confidential
> > > > On Tuesday, November 4, 2025 2:04 AM, Hannes Reinecke <hare@kernel.org> wrote:
> > > >>
> > > >> In some environments (eg kdump) not all CPUs are online, so the MQ
> > > >> mapping might be resulting in an invalid layout. So make the interrupt
> > > >> mode settable via an 'fnic_intr_mode' module parameter and switch
> > > >> to INTx if the 'reset_devices' kernel parameter is specified.
> > > >>
> > > >> Signed-off-by: Hannes Reinecke <hare@kernel.org>
> > > >> ---
> > > >> drivers/scsi/fnic/fnic.h      |  2 +-
> > > >> drivers/scsi/fnic/fnic_isr.c  | 13 +++++++++----
> > > >> drivers/scsi/fnic/fnic_main.c | 10 +++++++++-
> > > >> 3 files changed, 19 insertions(+), 6 deletions(-)
> > > >>
> > > >> diff --git a/drivers/scsi/fnic/fnic.h b/drivers/scsi/fnic/fnic.h
> > > >> index 1199d701c3f5..c679283955e9 100644
> > > >> --- a/drivers/scsi/fnic/fnic.h
> > > >> +++ b/drivers/scsi/fnic/fnic.h
> > > >> @@ -484,7 +484,7 @@ extern struct workqueue_struct *fnic_fip_queue;
> > > >> extern const struct attribute_group *fnic_host_groups[];
> > > >>
> > > >> void fnic_clear_intr_mode(struct fnic *fnic);
> > > >> -int fnic_set_intr_mode(struct fnic *fnic);
> > > >> +int fnic_set_intr_mode(struct fnic *fnic, unsigned int mode);
> > > >> int fnic_set_intr_mode_msix(struct fnic *fnic);
> > > >> void fnic_free_intr(struct fnic *fnic);
> > > >> int fnic_request_intr(struct fnic *fnic);
> > > >> diff --git a/drivers/scsi/fnic/fnic_isr.c b/drivers/scsi/fnic/fnic_isr.c
> > > >> index e16b76d537e8..b6594ad064ca 100644
> > > >> --- a/drivers/scsi/fnic/fnic_isr.c
> > > >> +++ b/drivers/scsi/fnic/fnic_isr.c
> > > >> @@ -319,20 +319,25 @@ int fnic_set_intr_mode_msix(struct fnic *fnic)
> > > >> return 1;
> > > >> }
> > > >>
> > > >> -int fnic_set_intr_mode(struct fnic *fnic)
> > > >> +int fnic_set_intr_mode(struct fnic *fnic, unsigned int intr_mode)
> > > >> {
> > > >> int ret_status = 0;
> > > >>
> > > >> /*
> > > >> * Set interrupt mode (INTx, MSI, MSI-X) depending
> > > >> * system capabilities.
> > > >> -      *
> > > >> +      */
> > > >> +     if (intr_mode != VNIC_DEV_INTR_MODE_MSIX)
> > > >> +             goto try_msi;
> > > >> +     /*
> > > >> * Try MSI-X first
> > > >> */
> > > >> ret_status = fnic_set_intr_mode_msix(fnic);
> > > >> if (ret_status == 0)
> > > >> return ret_status;
> > > >> -
> > > >> +try_msi:
> > > >> +     if (intr_mode != VNIC_DEV_INTR_MODE_MSI)
> > > >> +             goto try_intx;
> > > >> /*
> > > >> * Next try MSI
> > > >> * We need 1 RQ, 1 WQ, 1 WQ_COPY, 3 CQs, and 1 INTR
> > > >> @@ -358,7 +363,7 @@ int fnic_set_intr_mode(struct fnic *fnic)
> > > >>
> > > >> return 0;
> > > >> }
> > > >> -
> > > >> +try_intx:
> > > >> /*
> > > >> * Next try INTx
> > > >> * We need 1 RQ, 1 WQ, 1 WQ_COPY, 3 CQs, and 3 INTRs
> > > >> diff --git a/drivers/scsi/fnic/fnic_main.c b/drivers/scsi/fnic/fnic_main.c
> > > >> index 870b265be41a..4bdd55958f59 100644
> > > >> --- a/drivers/scsi/fnic/fnic_main.c
> > > >> +++ b/drivers/scsi/fnic/fnic_main.c
> > > >> @@ -97,6 +97,10 @@ module_param(pc_rscn_handling_feature_flag, uint, 0644);
> > > >> MODULE_PARM_DESC(pc_rscn_handling_feature_flag,
> > > >> "PCRSCN handling (0 for none. 1 to handle PCRSCN (default))");
> > > >>
> > > >> +static unsigned int fnic_intr_mode = VNIC_DEV_INTR_MODE_MSIX;
> > > >> +module_param(fnic_intr_mode, uint, S_IRUGO | S_IWUSR);
> > > >> +MODULE_PARM_DESC(fnic_intr_mode, "Interrupt mode, 1 = INTx, 2 = MSI, 3 = MSIx (default: 3)");
> > > >
> > > > Based on fnic team's review: there is a way to choose the interrupt mode using the UCS management platform.
> > > > We do not want to expose this as a module parameter.
> > > >
> > > Yeah, I know. It was primarily used during testing to easily change
> > > between the various modes.
> > > I can drop it for the next round.
> >
> > Thanks Hannes. Sounds good.
> >
> > > >> struct workqueue_struct *reset_fnic_work_queue;
> > > >> struct workqueue_struct *fnic_fip_queue;
> > > >>
> > > >> @@ -869,7 +873,11 @@ static int fnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > > >>
> > > >> fnic_get_res_counts(fnic);
> > > >>
> > > >> -     err = fnic_set_intr_mode(fnic);
> > > >> +     /* Override interrupt selection during kdump */
> > > >> +     if (reset_devices)
> > > >> +             fnic_intr_mode = VNIC_DEV_INTR_MODE_INTX;
> > > >> +
> > > >> +     err = fnic_set_intr_mode(fnic, fnic_intr_mode);
> > > >> if (err) {
> > > >> dev_err(&fnic->pdev->dev, "Failed to set intr mode, "
> > > >> "aborting.\n");
> > > >> --
> > > >> 2.43.0
> > > >>
> > > >>
> > > >
> > > > Thanks for these changes, Hannes.
> > > > For the other changes in this patch, I will need to test and get back to you.
> > > >
> > >
> > > Thanks.
> > > The 'reset_devices' thing is primarily there for kdump; might be an idea
> > > to make is explicit by using 'is_kdump_kernel()' directly.
> >
> > Yes Hannes. That would be better. Thanks.
> >
> > > Cheers,
> > >
> > > Hannes
> > >
> > > --
> > > Dr. Hannes Reinecke                  Kernel Storage Architect
> > > hare@suse.de                                +49 911 74053 688
> > > SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
> > > HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
> > >
> >
> > I'm bringing up a setup to test. I'll wait for your next revision.
> >
> > My plan is to induce a kdump to test out these changes.
> > If you have any other tests in mind, please let me know.
> >
> > Thanks,
> > Karan
> >
>
> Hi Hannes,
>
> Please consider sending out a new revision of these changes.
> I'd like to test all the changes and add a tested-by tag to them.
>
> Thanks,
> Karan
>

Hi Karan:

My name is Lee Duncan, and I work with Hannes. He is otherwise occupied
right now, so I'll do my best to take it from here.

It seems like you wanted two changes to patch#4:
1. Remove the newly-added module parameter, and
2. Use "is_kdump_kernel()" to test for setting the interrupt mode to INTX

I have made those changes to patch#4, and internal testing shows that
kdump works.

I will resubmit the patch sequence after one more internal test. I appreciate
your patience.

P.S, I believe the patches will end up getting to you through other channels,
since a bug was filed, but if you'd like to test them and want a copy before
I repost the sequence, please contact me.

RE: [PATCH 4/4] fnic: make interrupt mode configurable

[Karan Tilak Kumar (kartilak)]

On Friday, January 23, 2026 10:22 AM, Lee Duncan <lduncan@suse.com> wrote:
>
> On Wed, Jan 14, 2026 at 9:04 AM Karan Tilak Kumar (kartilak)
> <kartilak@cisco.com> wrote:
> >
> >
> > Cisco Confidential
> > On Friday, November 7, 2025 2:23 PM, Karan Tilak Kumar (kartilak) wrote:
> > >
> > > On Friday, November 7, 2025 12:29 AM, Hannes Reinecke <hare@suse.de> wrote:
> > > >
> > > > On 11/6/25 23:09, Karan Tilak Kumar (kartilak) wrote:
> > > > >
> > > > > Cisco Confidential
> > > > > On Tuesday, November 4, 2025 2:04 AM, Hannes Reinecke <hare@kernel.org> wrote:
> > > > >>
> > > > >> In some environments (eg kdump) not all CPUs are online, so the MQ
> > > > >> mapping might be resulting in an invalid layout. So make the interrupt
> > > > >> mode settable via an 'fnic_intr_mode' module parameter and switch
> > > > >> to INTx if the 'reset_devices' kernel parameter is specified.
> > > > >>
> > > > >> Signed-off-by: Hannes Reinecke <hare@kernel.org>
> > > > >> ---
> > > > >> drivers/scsi/fnic/fnic.h      |  2 +-
> > > > >> drivers/scsi/fnic/fnic_isr.c  | 13 +++++++++----
> > > > >> drivers/scsi/fnic/fnic_main.c | 10 +++++++++-
> > > > >> 3 files changed, 19 insertions(+), 6 deletions(-)
> > > > >>
> > > > >> diff --git a/drivers/scsi/fnic/fnic.h b/drivers/scsi/fnic/fnic.h
> > > > >> index 1199d701c3f5..c679283955e9 100644
> > > > >> --- a/drivers/scsi/fnic/fnic.h
> > > > >> +++ b/drivers/scsi/fnic/fnic.h
> > > > >> @@ -484,7 +484,7 @@ extern struct workqueue_struct *fnic_fip_queue;
> > > > >> extern const struct attribute_group *fnic_host_groups[];
> > > > >>
> > > > >> void fnic_clear_intr_mode(struct fnic *fnic);
> > > > >> -int fnic_set_intr_mode(struct fnic *fnic);
> > > > >> +int fnic_set_intr_mode(struct fnic *fnic, unsigned int mode);
> > > > >> int fnic_set_intr_mode_msix(struct fnic *fnic);
> > > > >> void fnic_free_intr(struct fnic *fnic);
> > > > >> int fnic_request_intr(struct fnic *fnic);
> > > > >> diff --git a/drivers/scsi/fnic/fnic_isr.c b/drivers/scsi/fnic/fnic_isr.c
> > > > >> index e16b76d537e8..b6594ad064ca 100644
> > > > >> --- a/drivers/scsi/fnic/fnic_isr.c
> > > > >> +++ b/drivers/scsi/fnic/fnic_isr.c
> > > > >> @@ -319,20 +319,25 @@ int fnic_set_intr_mode_msix(struct fnic *fnic)
> > > > >> return 1;
> > > > >> }
> > > > >>
> > > > >> -int fnic_set_intr_mode(struct fnic *fnic)
> > > > >> +int fnic_set_intr_mode(struct fnic *fnic, unsigned int intr_mode)
> > > > >> {
> > > > >> int ret_status = 0;
> > > > >>
> > > > >> /*
> > > > >> * Set interrupt mode (INTx, MSI, MSI-X) depending
> > > > >> * system capabilities.
> > > > >> -      *
> > > > >> +      */
> > > > >> +     if (intr_mode != VNIC_DEV_INTR_MODE_MSIX)
> > > > >> +             goto try_msi;
> > > > >> +     /*
> > > > >> * Try MSI-X first
> > > > >> */
> > > > >> ret_status = fnic_set_intr_mode_msix(fnic);
> > > > >> if (ret_status == 0)
> > > > >> return ret_status;
> > > > >> -
> > > > >> +try_msi:
> > > > >> +     if (intr_mode != VNIC_DEV_INTR_MODE_MSI)
> > > > >> +             goto try_intx;
> > > > >> /*
> > > > >> * Next try MSI
> > > > >> * We need 1 RQ, 1 WQ, 1 WQ_COPY, 3 CQs, and 1 INTR
> > > > >> @@ -358,7 +363,7 @@ int fnic_set_intr_mode(struct fnic *fnic)
> > > > >>
> > > > >> return 0;
> > > > >> }
> > > > >> -
> > > > >> +try_intx:
> > > > >> /*
> > > > >> * Next try INTx
> > > > >> * We need 1 RQ, 1 WQ, 1 WQ_COPY, 3 CQs, and 3 INTRs
> > > > >> diff --git a/drivers/scsi/fnic/fnic_main.c b/drivers/scsi/fnic/fnic_main.c
> > > > >> index 870b265be41a..4bdd55958f59 100644
> > > > >> --- a/drivers/scsi/fnic/fnic_main.c
> > > > >> +++ b/drivers/scsi/fnic/fnic_main.c
> > > > >> @@ -97,6 +97,10 @@ module_param(pc_rscn_handling_feature_flag, uint, 0644);
> > > > >> MODULE_PARM_DESC(pc_rscn_handling_feature_flag,
> > > > >> "PCRSCN handling (0 for none. 1 to handle PCRSCN (default))");
> > > > >>
> > > > >> +static unsigned int fnic_intr_mode = VNIC_DEV_INTR_MODE_MSIX;
> > > > >> +module_param(fnic_intr_mode, uint, S_IRUGO | S_IWUSR);
> > > > >> +MODULE_PARM_DESC(fnic_intr_mode, "Interrupt mode, 1 = INTx, 2 = MSI, 3 = MSIx (default: 3)");
> > > > >
> > > > > Based on fnic team's review: there is a way to choose the interrupt mode using the UCS management platform.
> > > > > We do not want to expose this as a module parameter.
> > > > >
> > > > Yeah, I know. It was primarily used during testing to easily change
> > > > between the various modes.
> > > > I can drop it for the next round.
> > >
> > > Thanks Hannes. Sounds good.
> > >
> > > > >> struct workqueue_struct *reset_fnic_work_queue;
> > > > >> struct workqueue_struct *fnic_fip_queue;
> > > > >>
> > > > >> @@ -869,7 +873,11 @@ static int fnic_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> > > > >>
> > > > >> fnic_get_res_counts(fnic);
> > > > >>
> > > > >> -     err = fnic_set_intr_mode(fnic);
> > > > >> +     /* Override interrupt selection during kdump */
> > > > >> +     if (reset_devices)
> > > > >> +             fnic_intr_mode = VNIC_DEV_INTR_MODE_INTX;
> > > > >> +
> > > > >> +     err = fnic_set_intr_mode(fnic, fnic_intr_mode);
> > > > >> if (err) {
> > > > >> dev_err(&fnic->pdev->dev, "Failed to set intr mode, "
> > > > >> "aborting.\n");
> > > > >> --
> > > > >> 2.43.0
> > > > >>
> > > > >>
> > > > >
> > > > > Thanks for these changes, Hannes.
> > > > > For the other changes in this patch, I will need to test and get back to you.
> > > > >
> > > >
> > > > Thanks.
> > > > The 'reset_devices' thing is primarily there for kdump; might be an idea
> > > > to make is explicit by using 'is_kdump_kernel()' directly.
> > >
> > > Yes Hannes. That would be better. Thanks.
> > >
> > > > Cheers,
> > > >
> > > > Hannes
> > > >
> > > > --
> > > > Dr. Hannes Reinecke                  Kernel Storage Architect
> > > > hare@suse.de                                +49 911 74053 688
> > > > SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
> > > > HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich
> > > >
> > >
> > > I'm bringing up a setup to test. I'll wait for your next revision.
> > >
> > > My plan is to induce a kdump to test out these changes.
> > > If you have any other tests in mind, please let me know.
> > >
> > > Thanks,
> > > Karan
> > >
> >
> > Hi Hannes,
> >
> > Please consider sending out a new revision of these changes.
> > I'd like to test all the changes and add a tested-by tag to them.
> >
> > Thanks,
> > Karan
> >
>
> Hi Karan:
>
> My name is Lee Duncan, and I work with Hannes. He is otherwise occupied
> right now, so I'll do my best to take it from here.
>
> It seems like you wanted two changes to patch#4:
> 1. Remove the newly-added module parameter, and
> 2. Use "is_kdump_kernel()" to test for setting the interrupt mode to INTX
>
> I have made those changes to patch#4, and internal testing shows that
> kdump works.
>

Hi Lee,

Appreciate your response and your willingness to take this forward.
Sounds good. Thank you for the changes and internal tests.

> I will resubmit the patch sequence after one more internal test. I appreciate
> your patience.
>
> P.S, I believe the patches will end up getting to you through other channels,
> since a bug was filed, but if you'd like to test them and want a copy before
> I repost the sequence, please contact me.
>

I understand. I would like to dev test these changes on top the latest kernel just to make sure things are working correctly.
I don't mind waiting until the internal tests are completed. I'll take your next revision in for testing.
Thanks again for your help.

Regards,
Karan