* [PATCH v4 0/8] ibmvfc: make ibmvfc support FPIN messages
@ 2026-07-10 19:16 Dave Marquardt via B4 Relay
2026-07-10 19:16 ` [PATCH v4 1/8] ibmvfc: add basic FPIN support Dave Marquardt via B4 Relay
` (7 more replies)
0 siblings, 8 replies; 17+ messages in thread
From: Dave Marquardt via B4 Relay @ 2026-07-10 19:16 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
Tyrel Datwyler
Cc: linux-kernel, linux-scsi, linuxppc-dev, Brian King, Greg Joyce,
Kyle Mahlkuch, Dave Marquardt
This patch series adds FPIN (fabric performance impact notification)
support to the ibmvfc (IBM Virtual Fibre Channel) driver. This comes
in three flavors:
- basic, to recognize existing FPIN messages from the virtual I/O
server (VIOS) (patch 1)
- full, supporting additional FPIN information and using its own
asynchronous sub-queue and interrupt (patches 4-7)
- extended, supporting FC-LS-5 (patch 8)
Full and extended FPIN support requires a new asynchronous sub-queue
with its own interrupt. The asynchronous sub-queue support requires
ibmvfc to also support
- a new VFC_NOOP command, which the driver recognizes and
ignores (patch 2)
- fabric login, to login separately to the fabric through messages
exchanged with VIOS rather than doing fabric login through the
existing NPIV login (patch 3)
All three modes convert an incoming FPIN message from VIOS to an FC
extended link service message, in some cases using default values for
information not provided by the VIOS FPIN message but expected in the
FC ELS message. This FC ELS message is passed to fc_host_rcv_fpin for
updating statistics and sending the information upstream by netlink
multicast, where it may be read by listeners including the DM
multipath daemon "multipathd."
Signed-off-by: Dave Marquardt <davemarq@linux.ibm.com>
---
Changes in v4:
- Refactored channel registration
- Check whether async work queue is allocated before using or freeing
- Fixed work queue allocation/destruction
- Skip basic KUnit test when there are no ibmvfc devices available
- Fix target not found condition in ibmvfc_process_async_work
- Link to v3: https://patch.msgid.link/20260702-ibmvfc-fpin-support-v3-0-d95b9547cf88@linux.ibm.com
Changes in v3:
- Fixed latent bug, exposed by VFC_NOOP, related to dataless CRQs and events
- Fixed FPIN TLV descriptor length calculations
- Use safe list walker to walk targets in ibmvfc_process_async_work
- Added write memory barriers after clearing CRQ valid field
- Use per-vhost work queue for FPIN work
- Link to v2: https://patch.msgid.link/20260608-ibmvfc-fpin-support-v2-0-d41f540fba5c@linux.ibm.com
Highlights of changes in v2:
- Refactored mostly common FPIN conversion routines and async event
processing into single routines with wrappers for differences.
- Moved FPIN processing to a work queue to avoid conflicts with
fc_host_fpin_rcv and memory allocation
- Set descriptor sizes correctly
- Use target WWPN for basic FPIN descriptor
- Split patch 4 into 3 patches, for definition, allocation, and use of
the asynchronous sub-queue for events
- Link to v1: https://patch.msgid.link/20260408-ibmvfc-fpin-support-v1-0-52b06c464e03@linux.ibm.com
---
Dave Marquardt (8):
ibmvfc: add basic FPIN support
ibmvfc: Add NOOP command support
ibmvfc: make ibmvfc login to fabric
ibmvfc: define asynchronous sub-queue
ibmvfc: allocate asynchronous sub-queue
ibmvfc: extend async event handlers to handle async sub queue events
ibmvfc: register and use asynchronous sub-queue for events
ibmvfc: handle extended FPIN events
drivers/scsi/Kconfig | 10 +
drivers/scsi/ibmvscsi/Makefile | 1 +
drivers/scsi/ibmvscsi/ibmvfc.c | 736 ++++++++++++++++++++++++++++++++---
drivers/scsi/ibmvscsi/ibmvfc.h | 115 +++++-
drivers/scsi/ibmvscsi/ibmvfc_kunit.c | 240 ++++++++++++
5 files changed, 1031 insertions(+), 71 deletions(-)
---
base-commit: 57a6ed0b41677ccc5e28cc0976e495c1dfa33747
change-id: 20260407-ibmvfc-fpin-support-b9b575cd2da1
Best regards,
--
Dave Marquardt <davemarq@linux.ibm.com>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCH v4 1/8] ibmvfc: add basic FPIN support
2026-07-10 19:16 [PATCH v4 0/8] ibmvfc: make ibmvfc support FPIN messages Dave Marquardt via B4 Relay
@ 2026-07-10 19:16 ` Dave Marquardt via B4 Relay
2026-07-10 19:31 ` sashiko-bot
2026-07-10 19:16 ` [PATCH v4 2/8] ibmvfc: Add NOOP command support Dave Marquardt via B4 Relay
` (6 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Dave Marquardt via B4 Relay @ 2026-07-10 19:16 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
Tyrel Datwyler
Cc: linux-kernel, linux-scsi, linuxppc-dev, Brian King, Greg Joyce,
Kyle Mahlkuch, Dave Marquardt
From: Dave Marquardt <davemarq@linux.ibm.com>
Implement support for a basic level of Fabric Performance Impact
Notifications (FPIN) in the ibmvfc driver to enable monitoring of
fabric congestion and link integrity events.
Add async event handler for IBMVFC_AE_FPIN events that offloads FPIN
processing to a dedicated workqueue. Convert VIOS FPIN messages to
standard fc_els_fpin structures and pass them to fc_host_fpin_rcv() for
processing by the FC transport layer.
Introduce common FPIN conversion routines that will be reused for full
and extended FPIN support in subsequent patches. Add KUnit test
infrastructure to validate FPIN event handling and statistics updates.
Changes include:
- Add FPIN async event handling in ibmvfc_handle_async()
- Create dedicated workqueue for FPIN processing
- Implement FPIN message conversion to fc_els_fpin format
- Add support for link congestion, port congestion, port cleared, port
degraded, and congestion cleared events
- Add KUnit test module for FPIN functionality
Signed-off-by: Dave Marquardt <davemarq@linux.ibm.com>
---
drivers/scsi/Kconfig | 10 ++
drivers/scsi/ibmvscsi/Makefile | 1 +
drivers/scsi/ibmvscsi/ibmvfc.c | 253 ++++++++++++++++++++++++++++++++++-
drivers/scsi/ibmvscsi/ibmvfc.h | 16 +++
drivers/scsi/ibmvscsi/ibmvfc_kunit.c | 132 ++++++++++++++++++
5 files changed, 405 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index c3042393af23..d5fc7eb2ebb1 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -758,6 +758,16 @@ config SCSI_IBMVFC
To compile this driver as a module, choose M here: the
module will be called ibmvfc.
+config SCSI_IBMVFC_KUNIT_TEST
+ tristate "KUnit tests for the IBM POWER Virtual FC Client" if !KUNIT_ALL_TESTS
+ depends on SCSI_IBMVFC && KUNIT
+ default KUNIT_ALL_TESTS
+ help
+ Compile IBM POWER Virtual FC client KUnit tests. These tests
+ specifically test FPIN functionality. To compile this driver
+ as a module, choose M here: the module will be called
+ ibmvfc_kunit.
+
config SCSI_IBMVFC_TRACE
bool "enable driver internal trace"
depends on SCSI_IBMVFC
diff --git a/drivers/scsi/ibmvscsi/Makefile b/drivers/scsi/ibmvscsi/Makefile
index 5eb1cb1a0028..75dc7aee15a0 100644
--- a/drivers/scsi/ibmvscsi/Makefile
+++ b/drivers/scsi/ibmvscsi/Makefile
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: GPL-2.0-only
obj-$(CONFIG_SCSI_IBMVSCSI) += ibmvscsi.o
obj-$(CONFIG_SCSI_IBMVFC) += ibmvfc.o
+obj-$(CONFIG_SCSI_IBMVFC_KUNIT_TEST) += ibmvfc_kunit.o
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 3dd2adda195e..d3fd1d3437c6 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -30,6 +30,9 @@
#include <scsi/scsi_tcq.h>
#include <scsi/scsi_transport_fc.h>
#include <scsi/scsi_bsg_fc.h>
+#include <kunit/visibility.h>
+#include <scsi/fc/fc_els.h>
+#include <linux/overflow.h>
#include "ibmvfc.h"
static unsigned int init_timeout = IBMVFC_INIT_TIMEOUT;
@@ -3137,6 +3140,7 @@ static const struct ibmvfc_async_desc ae_desc [] = {
{ "Halt", IBMVFC_AE_HALT, IBMVFC_DEFAULT_LOG_LEVEL },
{ "Resume", IBMVFC_AE_RESUME, IBMVFC_DEFAULT_LOG_LEVEL },
{ "Adapter Failed", IBMVFC_AE_ADAPTER_FAILED, IBMVFC_DEFAULT_LOG_LEVEL },
+ { "FPIN", IBMVFC_AE_FPIN, IBMVFC_DEFAULT_LOG_LEVEL },
};
static const struct ibmvfc_async_desc unknown_ae = {
@@ -3185,16 +3189,228 @@ static const char *ibmvfc_get_link_state(enum ibmvfc_ae_link_state state)
return "";
}
+#define IBMVFC_FPIN_CONGN_DESC_SZ (sizeof(struct fc_els_fpin) + sizeof(struct fc_fn_congn_desc))
+#define IBMVFC_FPIN_LI_DESC_SZ (sizeof(struct fc_els_fpin) + \
+ struct_size_t(struct fc_fn_li_desc, pname_list, 1))
+#define IBMVFC_FPIN_PEER_CONGN_DESC_SZ (sizeof(struct fc_els_fpin) + \
+ struct_size_t(struct fc_fn_peer_congn_desc, pname_list, 1))
+
+/**
+ * ibmvfc_fpin_size_helper(): compute fpin structure size based on fpin status
+ * @fpin_status: status value
+ *
+ * Return:
+ * 0: invalid fpin_status
+ * other: valid size
+ */
+static size_t ibmvfc_fpin_size_helper(u8 fpin_status)
+{
+ size_t size = 0;
+
+ switch (fpin_status) {
+ case IBMVFC_AE_FPIN_LINK_CONGESTED:
+ case IBMVFC_AE_FPIN_CONGESTION_CLEARED:
+ size = IBMVFC_FPIN_CONGN_DESC_SZ;
+ break;
+ case IBMVFC_AE_FPIN_PORT_CONGESTED:
+ case IBMVFC_AE_FPIN_PORT_CLEARED:
+ size = IBMVFC_FPIN_PEER_CONGN_DESC_SZ;
+ break;
+ case IBMVFC_AE_FPIN_PORT_DEGRADED:
+ size = IBMVFC_FPIN_LI_DESC_SZ;
+ break;
+ default:
+ break;
+ }
+
+ return size;
+}
+
+/**
+ * ibmvfc_common_fpin_to_desc(): allocate and populate a struct fc_els_fpin struct
+ * containing a descriptor.
+ *
+ * Allocate a struct fc_els_fpin containing a descriptor and populate
+ * based on data from *ibmvfc_fpin.
+ *
+ * Return:
+ * NULL - unable to allocate structure
+ * non-NULL - pointer to populated struct fc_els_fpin
+ */
+static struct fc_els_fpin *
+ibmvfc_common_fpin_to_desc(u8 fpin_status, __be64 wwpn, __be16 type, __be16 modifier,
+ __be32 threshold, __be32 event_count)
+{
+ struct fc_fn_peer_congn_desc *pdesc;
+ struct fc_fn_congn_desc *cdesc;
+ struct fc_fn_li_desc *ldesc;
+ struct fc_els_fpin *fpin;
+ size_t size;
+
+ size = ibmvfc_fpin_size_helper(fpin_status);
+ if (!size)
+ return NULL;
+
+ fpin = kzalloc(size, GFP_KERNEL);
+ if (!fpin)
+ return NULL;
+
+ fpin->fpin_cmd = ELS_FPIN;
+
+ switch (fpin_status) {
+ case IBMVFC_AE_FPIN_CONGESTION_CLEARED:
+ case IBMVFC_AE_FPIN_LINK_CONGESTED:
+ fpin->desc_len = cpu_to_be32(sizeof(struct fc_fn_congn_desc));
+ cdesc = (struct fc_fn_congn_desc *)fpin->fpin_desc;
+ cdesc->desc_tag = cpu_to_be32(ELS_DTAG_CONGESTION);
+ cdesc->desc_len = cpu_to_be32(FC_TLV_DESC_LENGTH_FROM_SZ(*cdesc));
+ cdesc->event_type = type;
+ cdesc->event_modifier = modifier;
+ cdesc->event_period = cpu_to_be32(IBMVFC_FPIN_DEFAULT_EVENT_PERIOD);
+ cdesc->severity = FPIN_CONGN_SEVERITY_WARNING;
+ break;
+ case IBMVFC_AE_FPIN_PORT_CONGESTED:
+ case IBMVFC_AE_FPIN_PORT_CLEARED:
+ fpin->desc_len =
+ cpu_to_be32(struct_size_t(struct fc_fn_peer_congn_desc, pname_list, 1));
+ pdesc = (struct fc_fn_peer_congn_desc *)fpin->fpin_desc;
+ pdesc->desc_tag = cpu_to_be32(ELS_DTAG_PEER_CONGEST);
+ pdesc->desc_len = cpu_to_be32(struct_size_t(struct fc_fn_peer_congn_desc,
+ pname_list, 1) - FC_TLV_DESC_HDR_SZ);
+ pdesc->event_type = type;
+ pdesc->event_modifier = modifier;
+ pdesc->event_period = cpu_to_be32(IBMVFC_FPIN_DEFAULT_EVENT_PERIOD);
+ pdesc->attached_wwpn = wwpn;
+ pdesc->pname_count = cpu_to_be32(1);
+ pdesc->pname_list[0] = wwpn;
+ break;
+ case IBMVFC_AE_FPIN_PORT_DEGRADED:
+ fpin->desc_len = cpu_to_be32(struct_size_t(struct fc_fn_li_desc, pname_list, 1));
+ ldesc = (struct fc_fn_li_desc *)fpin->fpin_desc;
+ ldesc->desc_tag = cpu_to_be32(ELS_DTAG_LNK_INTEGRITY);
+ ldesc->desc_len = cpu_to_be32(struct_size_t(struct fc_fn_li_desc,
+ pname_list, 1) - FC_TLV_DESC_HDR_SZ);
+ ldesc->event_type = type;
+ ldesc->event_modifier = modifier;
+ ldesc->event_threshold = threshold;
+ ldesc->event_count = event_count;
+ ldesc->attached_wwpn = wwpn;
+ ldesc->pname_count = cpu_to_be32(1);
+ ldesc->pname_list[0] = wwpn;
+ break;
+ default:
+ /* This should be caught above. */
+ kfree(fpin);
+ fpin = NULL;
+ break;
+ }
+
+ return fpin;
+}
+
+/**
+ * ibmvfc_basic_fpin_to_desc(): allocate and populate a struct fc_els_fpin struct
+ * containing a descriptor.
+ * @ibmvfc_fpin: Pointer to async crq
+ *
+ * Allocate a struct fc_els_fpin containing a descriptor and populate
+ * based on data from *ibmvfc_fpin.
+ *
+ * Return:
+ * NULL - unable to allocate structure
+ * non-NULL - pointer to populated struct fc_els_fpin
+ */
+static struct fc_els_fpin *
+ibmvfc_basic_fpin_to_desc(struct ibmvfc_async_crq *crq, u64 wwpn)
+{
+ __be16 type;
+
+ switch (crq->fpin_status) {
+ case IBMVFC_AE_FPIN_LINK_CONGESTED:
+ case IBMVFC_AE_FPIN_PORT_CONGESTED:
+ type = cpu_to_be16(FPIN_CONGN_DEVICE_SPEC);
+ break;
+ case IBMVFC_AE_FPIN_PORT_CLEARED:
+ case IBMVFC_AE_FPIN_CONGESTION_CLEARED:
+ type = cpu_to_be16(FPIN_CONGN_CLEAR);
+ break;
+ case IBMVFC_AE_FPIN_PORT_DEGRADED:
+ type = cpu_to_be16(FPIN_LI_UNKNOWN);
+ break;
+ default:
+ return (NULL);
+ }
+
+ return ibmvfc_common_fpin_to_desc(crq->fpin_status, cpu_to_be64(wwpn),
+ type, cpu_to_be16(0),
+ cpu_to_be32(IBMVFC_FPIN_DEFAULT_EVENT_THRESHOLD),
+ cpu_to_be32(1));
+}
+
+/**
+ * ibmvfc_process_async_work - Process IBMVFC_AE_FPIN async CRQ from work queue
+ * @work: pointer to work_struct
+ */
+static void ibmvfc_process_async_work(struct work_struct *work)
+{
+ struct ibmvfc_target *tgt, *next;
+ struct ibmvfc_async_work *aw;
+ struct ibmvfc_async_crq *crq;
+ struct ibmvfc_host *vhost;
+ struct fc_els_fpin *fpin;
+ unsigned long flags;
+
+ aw = container_of(work, struct ibmvfc_async_work, async_work_s);
+ crq = &aw->crq;
+ vhost = aw->vhost;
+
+ if (!crq->scsi_id && !crq->wwpn && !crq->node_name)
+ goto end;
+
+ spin_lock_irqsave(vhost->host->host_lock, flags);
+ list_for_each_entry_safe(tgt, next, &vhost->targets, queue) {
+ if (crq->scsi_id && cpu_to_be64(tgt->scsi_id) != crq->scsi_id)
+ continue;
+ if (crq->wwpn && cpu_to_be64(tgt->ids.port_name) != crq->wwpn)
+ continue;
+ if (crq->node_name && cpu_to_be64(tgt->ids.node_name) != crq->node_name)
+ continue;
+ if (!tgt->rport)
+ continue;
+ break;
+ }
+ spin_unlock_irqrestore(vhost->host->host_lock, flags);
+
+ if (list_entry_is_head(tgt, &vhost->targets, queue) || !tgt->rport) {
+ dev_err_ratelimited(vhost->dev, "Invalid target for FPIN\n");
+ goto end;
+ }
+
+ fpin = ibmvfc_basic_fpin_to_desc(crq, tgt->wwpn);
+ if (fpin) {
+ fc_host_fpin_rcv(tgt->vhost->host,
+ sizeof(*fpin) + be32_to_cpu(fpin->desc_len),
+ (char *)fpin, 0);
+ kfree(fpin);
+ } else
+ dev_err_ratelimited(vhost->dev,
+ "FPIN event %u received, unable to process\n",
+ crq->fpin_status);
+ end:
+ kfree(aw);
+}
+
/**
* ibmvfc_handle_async - Handle an async event from the adapter
* @crq: crq to process
* @vhost: ibmvfc host struct
*
**/
-static void ibmvfc_handle_async(struct ibmvfc_async_crq *crq,
- struct ibmvfc_host *vhost)
+VISIBLE_IF_KUNIT void ibmvfc_handle_async(struct ibmvfc_async_crq *crq,
+ struct ibmvfc_host *vhost)
{
const struct ibmvfc_async_desc *desc = ibmvfc_get_ae_desc(be64_to_cpu(crq->event));
+ struct ibmvfc_async_work *aw;
struct ibmvfc_target *tgt;
ibmvfc_log(vhost, desc->log_level, "%s event received. scsi_id: %llx, wwpn: %llx,"
@@ -3269,11 +3485,23 @@ static void ibmvfc_handle_async(struct ibmvfc_async_crq *crq,
case IBMVFC_AE_HALT:
ibmvfc_link_down(vhost, IBMVFC_HALTED);
break;
+ case IBMVFC_AE_FPIN:
+ aw = kzalloc(sizeof(struct ibmvfc_async_work), GFP_ATOMIC);
+ if (aw) {
+ INIT_WORK(&aw->async_work_s, ibmvfc_process_async_work);
+ aw->vhost = vhost;
+ aw->crq = *crq;
+ queue_work(vhost->fpin_workq, &aw->async_work_s);
+ } else
+ dev_err_ratelimited(vhost->dev,
+ "can't offload async CRQ to work queue\n");
+ break;
default:
dev_err(vhost->dev, "Unknown async event received: %lld\n", crq->event);
break;
}
}
+EXPORT_SYMBOL_IF_KUNIT(ibmvfc_handle_async);
/**
* ibmvfc_handle_crq - Handles and frees received events in the CRQ
@@ -3803,8 +4031,6 @@ static void ibmvfc_tasklet(void *data)
/* Pull all the valid messages off the async CRQ */
while ((async = ibmvfc_next_async_crq(vhost)) != NULL) {
ibmvfc_handle_async(async, vhost);
- async->valid = 0;
- wmb();
}
/* Pull all the valid messages off the CRQ */
@@ -3818,8 +4044,6 @@ static void ibmvfc_tasklet(void *data)
if ((async = ibmvfc_next_async_crq(vhost)) != NULL) {
vio_disable_interrupts(vdev);
ibmvfc_handle_async(async, vhost);
- async->valid = 0;
- wmb();
} else if ((crq = ibmvfc_next_crq(vhost)) != NULL) {
vio_disable_interrupts(vdev);
ibmvfc_handle_crq(crq, vhost, &evt_doneq);
@@ -6364,9 +6588,15 @@ static int ibmvfc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
INIT_WORK(&vhost->rport_add_work_q, ibmvfc_rport_add_thread);
mutex_init(&vhost->passthru_mutex);
- if ((rc = ibmvfc_alloc_mem(vhost)))
+ vhost->fpin_workq = devm_alloc_workqueue(vhost->dev, "%s-fpin-workq-%u", 0, 0,
+ IBMVFC_NAME, shost->host_no);
+ if (vhost->fpin_workq == NULL)
goto free_scsi_host;
+ rc = ibmvfc_alloc_mem(vhost);
+ if (rc)
+ goto free_workq;
+
vhost->work_thread = kthread_run(ibmvfc_work, vhost, "%s_%d", IBMVFC_NAME,
shost->host_no);
@@ -6412,6 +6642,9 @@ static int ibmvfc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
kthread_stop(vhost->work_thread);
free_host_mem:
ibmvfc_free_mem(vhost);
+free_workq:
+ destroy_workqueue(vhost->fpin_workq);
+ vhost->fpin_workq = NULL;
free_scsi_host:
scsi_host_put(shost);
out:
@@ -6603,5 +6836,11 @@ static void __exit ibmvfc_module_exit(void)
fc_release_transport(ibmvfc_transport_template);
}
+VISIBLE_IF_KUNIT struct list_head *ibmvfc_get_headp(void)
+{
+ return &ibmvfc_head;
+}
+EXPORT_SYMBOL_IF_KUNIT(ibmvfc_get_headp);
+
module_init(ibmvfc_module_init);
module_exit(ibmvfc_module_exit);
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.h b/drivers/scsi/ibmvscsi/ibmvfc.h
index c73ed2314ad0..f69e0605a78d 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.h
+++ b/drivers/scsi/ibmvscsi/ibmvfc.h
@@ -671,8 +671,12 @@ enum ibmvfc_ae_fpin_status {
IBMVFC_AE_FPIN_PORT_CONGESTED = 0x2,
IBMVFC_AE_FPIN_PORT_CLEARED = 0x3,
IBMVFC_AE_FPIN_PORT_DEGRADED = 0x4,
+ IBMVFC_AE_FPIN_CONGESTION_CLEARED = 0x5,
};
+#define IBMVFC_FPIN_DEFAULT_EVENT_PERIOD (5*60*MSEC_PER_SEC) /* 5 minutes */
+#define IBMVFC_FPIN_DEFAULT_EVENT_THRESHOLD (5*60*MSEC_PER_SEC/2) /* 2.5 minutes */
+
struct ibmvfc_async_crq {
volatile u8 valid;
u8 link_state;
@@ -686,6 +690,12 @@ struct ibmvfc_async_crq {
__be64 reserved;
} __packed __aligned(8);
+struct ibmvfc_async_work {
+ struct ibmvfc_host *vhost;
+ struct ibmvfc_async_crq crq;
+ struct work_struct async_work_s;
+};
+
union ibmvfc_iu {
struct ibmvfc_mad_common mad_common;
struct ibmvfc_npiv_login_mad npiv_login;
@@ -914,6 +924,7 @@ struct ibmvfc_host {
struct work_struct rport_add_work_q;
wait_queue_head_t init_wait_q;
wait_queue_head_t work_wait_q;
+ struct workqueue_struct *fpin_workq;
};
#define DBG_CMD(CMD) do { if (ibmvfc_debug) CMD; } while (0)
@@ -953,4 +964,9 @@ struct ibmvfc_host {
#define ibmvfc_remove_trace_file(kobj, attr) do { } while (0)
#endif
+#ifdef VISIBLE_IF_KUNIT
+VISIBLE_IF_KUNIT void ibmvfc_handle_async(struct ibmvfc_async_crq *crq, struct ibmvfc_host *vhost);
+VISIBLE_IF_KUNIT struct list_head *ibmvfc_get_headp(void);
+#endif
+
#endif
diff --git a/drivers/scsi/ibmvscsi/ibmvfc_kunit.c b/drivers/scsi/ibmvscsi/ibmvfc_kunit.c
new file mode 100644
index 000000000000..1c90318b6811
--- /dev/null
+++ b/drivers/scsi/ibmvscsi/ibmvfc_kunit.c
@@ -0,0 +1,132 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+#include <kunit/test.h>
+#include <kunit/visibility.h>
+#include <scsi/scsi_device.h>
+#include <scsi/scsi_transport_fc.h>
+#include <linux/list.h>
+#include <linux/delay.h>
+#include "ibmvfc.h"
+
+MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
+
+/**
+ * ibmvfc_async_fpin_event_test - unit test for IBMVFC_AE_FPIN parts of
+ * ibmvfc_handle_async
+ * @test: pointer to kunit structure
+ *
+ * Tests
+ * - error returns from ibmvfc_handle_async
+ * - statistics updates
+ *
+ * Return: void
+ */
+static void ibmvfc_async_fpin_test(struct kunit *test)
+{
+ u64 post[IBMVFC_AE_FPIN_CONGESTION_CLEARED + 1];
+ u64 pre[IBMVFC_AE_FPIN_CONGESTION_CLEARED + 1];
+ enum ibmvfc_ae_fpin_status fs;
+ struct fc_host_attrs *fc_host;
+ struct ibmvfc_async_crq crq[IBMVFC_AE_FPIN_CONGESTION_CLEARED + 1];
+ struct ibmvfc_target *tgt;
+ struct ibmvfc_host *vhost;
+ struct list_head *queue;
+ struct list_head *headp;
+
+ headp = ibmvfc_get_headp();
+ if (list_empty(headp))
+ kunit_skip(test, "No ibmvfc devices available");
+ queue = headp->next;
+ vhost = container_of(queue, struct ibmvfc_host, queue);
+
+ KUNIT_ASSERT_GE_MSG(test, vhost->num_targets, 1, "No targets");
+ tgt = list_first_entry(&vhost->targets, struct ibmvfc_target, queue);
+ KUNIT_EXPECT_NOT_NULL(test, tgt->rport);
+
+ fc_host = shost_to_fc_host(vhost->host);
+
+ pre[IBMVFC_AE_FPIN_LINK_CONGESTED] = READ_ONCE(fc_host->fpin_stats.cn_device_specific);
+ pre[IBMVFC_AE_FPIN_PORT_CONGESTED] = READ_ONCE(tgt->rport->fpin_stats.cn);
+ pre[IBMVFC_AE_FPIN_PORT_CLEARED] = READ_ONCE(tgt->rport->fpin_stats.cn_clear);
+ pre[IBMVFC_AE_FPIN_PORT_DEGRADED] = READ_ONCE(tgt->rport->fpin_stats.li_failure_unknown);
+ pre[IBMVFC_AE_FPIN_CONGESTION_CLEARED] = READ_ONCE(fc_host->fpin_stats.cn_clear);
+
+ for (fs = IBMVFC_AE_FPIN_LINK_CONGESTED; fs <= IBMVFC_AE_FPIN_CONGESTION_CLEARED; fs++) {
+ crq[fs].valid = 0x80;
+ crq[fs].link_state = IBMVFC_AE_LS_LINK_UP;
+ crq[fs].fpin_status = fs;
+ crq[fs].event = cpu_to_be64(IBMVFC_AE_FPIN);
+ crq[fs].scsi_id = cpu_to_be64(tgt->scsi_id);
+ crq[fs].wwpn = cpu_to_be64(tgt->wwpn);
+ crq[fs].node_name = cpu_to_be64(tgt->ids.node_name);
+ ibmvfc_handle_async(&crq[fs], vhost);
+ }
+
+ msleep(500U);
+
+ post[IBMVFC_AE_FPIN_LINK_CONGESTED] = READ_ONCE(fc_host->fpin_stats.cn_device_specific);
+ post[IBMVFC_AE_FPIN_PORT_CONGESTED] = READ_ONCE(tgt->rport->fpin_stats.cn);
+ post[IBMVFC_AE_FPIN_PORT_CLEARED] = READ_ONCE(tgt->rport->fpin_stats.cn_clear);
+ post[IBMVFC_AE_FPIN_PORT_DEGRADED] = READ_ONCE(tgt->rport->fpin_stats.li_failure_unknown);
+ post[IBMVFC_AE_FPIN_CONGESTION_CLEARED] = READ_ONCE(fc_host->fpin_stats.cn_clear);
+
+ KUNIT_EXPECT_GE(test, post[IBMVFC_AE_FPIN_LINK_CONGESTED],
+ pre[IBMVFC_AE_FPIN_LINK_CONGESTED]+1);
+ KUNIT_EXPECT_GE(test, post[IBMVFC_AE_FPIN_PORT_CONGESTED],
+ pre[IBMVFC_AE_FPIN_PORT_CONGESTED]+1);
+ KUNIT_EXPECT_GE(test, post[IBMVFC_AE_FPIN_PORT_CLEARED],
+ pre[IBMVFC_AE_FPIN_PORT_CLEARED]+1);
+ KUNIT_EXPECT_GE(test, post[IBMVFC_AE_FPIN_PORT_DEGRADED],
+ pre[IBMVFC_AE_FPIN_PORT_DEGRADED]+1);
+ KUNIT_EXPECT_GE(test, post[IBMVFC_AE_FPIN_CONGESTION_CLEARED],
+ pre[IBMVFC_AE_FPIN_CONGESTION_CLEARED]+1);
+
+ pre[IBMVFC_AE_FPIN_LINK_CONGESTED] = READ_ONCE(fc_host->fpin_stats.cn_device_specific);
+ pre[IBMVFC_AE_FPIN_PORT_CONGESTED] = READ_ONCE(tgt->rport->fpin_stats.cn);
+ pre[IBMVFC_AE_FPIN_PORT_CLEARED] = READ_ONCE(tgt->rport->fpin_stats.cn_clear);
+ pre[IBMVFC_AE_FPIN_PORT_DEGRADED] = READ_ONCE(tgt->rport->fpin_stats.li_failure_unknown);
+ pre[IBMVFC_AE_FPIN_CONGESTION_CLEARED] = READ_ONCE(fc_host->fpin_stats.cn_clear);
+
+ /* bad path */
+ crq[0].valid = 0x80;
+ crq[0].link_state = IBMVFC_AE_LS_LINK_UP;
+ crq[0].fpin_status = 0; /* bad value */
+ crq[0].event = cpu_to_be64(IBMVFC_AE_FPIN);
+ crq[0].scsi_id = cpu_to_be64(tgt->scsi_id);
+ crq[0].wwpn = cpu_to_be64(tgt->wwpn);
+ crq[0].node_name = cpu_to_be64(tgt->ids.node_name);
+ ibmvfc_handle_async(&crq[0], vhost);
+
+ msleep(500U);
+
+ post[IBMVFC_AE_FPIN_LINK_CONGESTED] = READ_ONCE(fc_host->fpin_stats.cn_device_specific);
+ post[IBMVFC_AE_FPIN_PORT_CONGESTED] = READ_ONCE(tgt->rport->fpin_stats.cn);
+ post[IBMVFC_AE_FPIN_PORT_CLEARED] = READ_ONCE(tgt->rport->fpin_stats.cn_clear);
+ post[IBMVFC_AE_FPIN_PORT_DEGRADED] = READ_ONCE(tgt->rport->fpin_stats.li_failure_unknown);
+ post[IBMVFC_AE_FPIN_CONGESTION_CLEARED] = READ_ONCE(fc_host->fpin_stats.cn_clear);
+
+ KUNIT_EXPECT_EQ(test, pre[IBMVFC_AE_FPIN_LINK_CONGESTED],
+ post[IBMVFC_AE_FPIN_LINK_CONGESTED]);
+ KUNIT_EXPECT_EQ(test, pre[IBMVFC_AE_FPIN_PORT_CONGESTED],
+ post[IBMVFC_AE_FPIN_PORT_CONGESTED]);
+ KUNIT_EXPECT_EQ(test, pre[IBMVFC_AE_FPIN_PORT_CLEARED],
+ post[IBMVFC_AE_FPIN_PORT_CLEARED]);
+ KUNIT_EXPECT_EQ(test, pre[IBMVFC_AE_FPIN_PORT_DEGRADED],
+ post[IBMVFC_AE_FPIN_PORT_DEGRADED]);
+ KUNIT_EXPECT_EQ(test, pre[IBMVFC_AE_FPIN_CONGESTION_CLEARED],
+ post[IBMVFC_AE_FPIN_CONGESTION_CLEARED]);
+}
+
+static struct kunit_case ibmvfc_fpin_test_cases[] = {
+ KUNIT_CASE_SLOW(ibmvfc_async_fpin_test),
+ {},
+};
+
+static struct kunit_suite ibmvfc_fpin_test_suite = {
+ .name = "ibmvfc-fpin-test",
+ .test_cases = ibmvfc_fpin_test_cases,
+};
+kunit_test_init_section_suite(ibmvfc_fpin_test_suite);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Dave Marquardt <davemarq@linux.ibm.com>");
+MODULE_DESCRIPTION("Test module for IBM Virtual Fibre Channel Driver");
--
2.55.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v4 2/8] ibmvfc: Add NOOP command support
2026-07-10 19:16 [PATCH v4 0/8] ibmvfc: make ibmvfc support FPIN messages Dave Marquardt via B4 Relay
2026-07-10 19:16 ` [PATCH v4 1/8] ibmvfc: add basic FPIN support Dave Marquardt via B4 Relay
@ 2026-07-10 19:16 ` Dave Marquardt via B4 Relay
2026-07-10 19:31 ` sashiko-bot
2026-07-10 19:16 ` [PATCH v4 3/8] ibmvfc: make ibmvfc login to fabric Dave Marquardt via B4 Relay
` (5 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Dave Marquardt via B4 Relay @ 2026-07-10 19:16 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
Tyrel Datwyler
Cc: linux-kernel, linux-scsi, linuxppc-dev, Brian King, Greg Joyce,
Kyle Mahlkuch, Dave Marquardt
From: Dave Marquardt <davemarq@linux.ibm.com>
Add support for VFC_NOOP messages from VIOS to enable keep-alive
functionality between the client and server.
Define the VFC_NOOP CRQ format and add handling in both the main CRQ
handler (ibmvfc_handle_crq) and sub-CRQ handler (ibmvfc_handle_scrq).
Log unexpected NOOP messages if received before VIOS advertises support
during NPIV login.
Set the IBMVFC_CAN_USE_NOOP_CMD capability bit during NPIV login to
inform VIOS that the client can handle NOOP commands.
Signed-off-by: Dave Marquardt <davemarq@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 22 ++++++++++++++++++++--
drivers/scsi/ibmvscsi/ibmvfc.h | 23 +++++++++++++----------
2 files changed, 33 insertions(+), 12 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index d3fd1d3437c6..a7e3b7ee0683 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -1512,7 +1512,9 @@ static void ibmvfc_set_login_info(struct ibmvfc_host *vhost)
login_info->flags |= cpu_to_be16(IBMVFC_CLIENT_MIGRATED);
login_info->max_cmds = cpu_to_be32(max_cmds);
- login_info->capabilities = cpu_to_be64(IBMVFC_CAN_MIGRATE | IBMVFC_CAN_SEND_VF_WWPN);
+ login_info->capabilities =
+ cpu_to_be64(IBMVFC_CAN_MIGRATE | IBMVFC_CAN_SEND_VF_WWPN |
+ IBMVFC_CAN_USE_NOOP_CMD);
if (vhost->mq_enabled || vhost->using_channels)
login_info->capabilities |= cpu_to_be64(IBMVFC_CAN_USE_CHANNELS);
@@ -3569,6 +3571,14 @@ static void ibmvfc_handle_crq(struct ibmvfc_crq *crq, struct ibmvfc_host *vhost,
if (crq->format == IBMVFC_ASYNC_EVENT)
return;
+ if (crq->format == IBMVFC_VFC_NOOP) {
+ if (vhost->state == IBMVFC_ACTIVE &&
+ !ibmvfc_check_caps(vhost, IBMVFC_SUPPORT_NOOP_CMD))
+ dev_err_ratelimited(vhost->dev,
+ "Received unexpected NOOP command from partner\n");
+ return;
+ }
+
/* The only kind of payload CRQs we should get are responses to
* things we send. Make sure this response is to something we
* actually sent
@@ -4095,7 +4105,15 @@ static void ibmvfc_handle_scrq(struct ibmvfc_crq *crq, struct ibmvfc_host *vhost
case IBMVFC_CRQ_XPORT_EVENT:
return;
default:
- dev_err(vhost->dev, "Got and invalid message type 0x%02x\n", crq->valid);
+ dev_err(vhost->dev, "Got an invalid message type 0x%02x\n", crq->valid);
+ return;
+ }
+
+ if (crq->format == IBMVFC_VFC_NOOP)
+ return;
+
+ if (unlikely(!evt)) {
+ dev_err(vhost->dev, "Received null event\n");
return;
}
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.h b/drivers/scsi/ibmvscsi/ibmvfc.h
index f69e0605a78d..526632cb7237 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.h
+++ b/drivers/scsi/ibmvscsi/ibmvfc.h
@@ -175,11 +175,12 @@ struct ibmvfc_npiv_login {
#define IBMVFC_FLUSH_ON_HALT 0x02
__be32 max_cmds;
__be64 capabilities;
-#define IBMVFC_CAN_MIGRATE 0x01
-#define IBMVFC_CAN_USE_CHANNELS 0x02
-#define IBMVFC_CAN_HANDLE_FPIN 0x04
-#define IBMVFC_CAN_USE_MAD_VERSION 0x08
-#define IBMVFC_CAN_SEND_VF_WWPN 0x10
+#define IBMVFC_CAN_MIGRATE 0x001
+#define IBMVFC_CAN_USE_CHANNELS 0x002
+#define IBMVFC_CAN_HANDLE_FPIN 0x004
+#define IBMVFC_CAN_USE_MAD_VERSION 0x008
+#define IBMVFC_CAN_SEND_VF_WWPN 0x010
+#define IBMVFC_CAN_USE_NOOP_CMD 0x200
__be64 node_name;
struct srp_direct_buf async;
u8 partition_name[IBMVFC_MAX_NAME];
@@ -221,11 +222,12 @@ struct ibmvfc_npiv_login_resp {
#define IBMVFC_NATIVE_FC 0x01
__be32 reserved;
__be64 capabilities;
-#define IBMVFC_CAN_FLUSH_ON_HALT 0x08
-#define IBMVFC_CAN_SUPPRESS_ABTS 0x10
-#define IBMVFC_MAD_VERSION_CAP 0x20
-#define IBMVFC_HANDLE_VF_WWPN 0x40
-#define IBMVFC_CAN_SUPPORT_CHANNELS 0x80
+#define IBMVFC_CAN_FLUSH_ON_HALT 0x0008
+#define IBMVFC_CAN_SUPPRESS_ABTS 0x0010
+#define IBMVFC_MAD_VERSION_CAP 0x0020
+#define IBMVFC_HANDLE_VF_WWPN 0x0040
+#define IBMVFC_CAN_SUPPORT_CHANNELS 0x0080
+#define IBMVFC_SUPPORT_NOOP_CMD 0x1000
__be32 max_cmds;
__be32 scsi_id_sz;
__be64 max_dma_len;
@@ -621,6 +623,7 @@ struct ibmvfc_trace_entry {
enum ibmvfc_crq_formats {
IBMVFC_CMD_FORMAT = 0x01,
IBMVFC_ASYNC_EVENT = 0x02,
+ IBMVFC_VFC_NOOP = 0x03,
IBMVFC_MAD_FORMAT = 0x04,
};
--
2.55.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v4 3/8] ibmvfc: make ibmvfc login to fabric
2026-07-10 19:16 [PATCH v4 0/8] ibmvfc: make ibmvfc support FPIN messages Dave Marquardt via B4 Relay
2026-07-10 19:16 ` [PATCH v4 1/8] ibmvfc: add basic FPIN support Dave Marquardt via B4 Relay
2026-07-10 19:16 ` [PATCH v4 2/8] ibmvfc: Add NOOP command support Dave Marquardt via B4 Relay
@ 2026-07-10 19:16 ` Dave Marquardt via B4 Relay
2026-07-10 19:30 ` sashiko-bot
2026-07-10 19:16 ` [PATCH v4 4/8] ibmvfc: define asynchronous sub-queue Dave Marquardt via B4 Relay
` (4 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Dave Marquardt via B4 Relay @ 2026-07-10 19:16 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
Tyrel Datwyler
Cc: linux-kernel, linux-scsi, linuxppc-dev, Brian King, Greg Joyce,
Kyle Mahlkuch, Dave Marquardt
From: Dave Marquardt <davemarq@linux.ibm.com>
Add fabric login capability to support asynchronous event queue with
dedicated interrupt as required by NPIV specification for async sub-queue
and full/extended FPIN message support.
Implement ibmvfc_fabric_login() to perform fabric login using MAD
(Management Adapter Data) format. Add ibmvfc_fabric_login_done() callback
to handle login completion, including error handling and retry logic.
On successful fabric login, store the assigned N_Port ID in the FC host
structure and transition to IBMVFC_HOST_ACTION_QUERY state to continue
initialization.
Signed-off-by: Dave Marquardt <davemarq@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 96 ++++++++++++++++++++++++++++++++++++++++--
drivers/scsi/ibmvscsi/ibmvfc.h | 17 ++++++++
2 files changed, 109 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index a7e3b7ee0683..c95e78d729ed 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -5265,6 +5265,88 @@ static void ibmvfc_discover_targets(struct ibmvfc_host *vhost)
ibmvfc_link_down(vhost, IBMVFC_LINK_DEAD);
}
+static void ibmvfc_fabric_login_done(struct ibmvfc_event *evt)
+{
+ struct ibmvfc_fabric_login *rsp = &evt->xfer_iu->fabric_login;
+ u32 mad_status = be16_to_cpu(rsp->common.status);
+ struct ibmvfc_host *vhost = evt->vhost;
+ int level = IBMVFC_DEFAULT_LOG_LEVEL;
+
+ ENTER;
+
+ switch (mad_status) {
+ case IBMVFC_MAD_SUCCESS:
+ fc_host_port_id(vhost->host) = be64_to_cpu(rsp->nport_id);
+ ibmvfc_free_event(evt);
+ break;
+
+ case IBMVFC_MAD_FAILED:
+ if (ibmvfc_retry_cmd(be16_to_cpu(rsp->status), be16_to_cpu(rsp->error)))
+ level += ibmvfc_retry_host_init(vhost);
+ else
+ ibmvfc_link_down(vhost, IBMVFC_LINK_DEAD);
+ ibmvfc_log(vhost, level, "Fabric Login failed: %s (%x:%x)\n",
+ ibmvfc_get_cmd_error(be16_to_cpu(rsp->status), be16_to_cpu(rsp->error)),
+ be16_to_cpu(rsp->status), be16_to_cpu(rsp->error));
+ ibmvfc_free_event(evt);
+ LEAVE;
+ return;
+
+ case IBMVFC_MAD_CRQ_ERROR:
+ ibmvfc_retry_host_init(vhost);
+ fallthrough;
+
+ case IBMVFC_MAD_DRIVER_FAILED:
+ ibmvfc_free_event(evt);
+ LEAVE;
+ return;
+
+ default:
+ dev_err(vhost->dev, "Invalid fabric Login response: 0x%x\n", mad_status);
+ ibmvfc_link_down(vhost, IBMVFC_LINK_DEAD);
+ ibmvfc_free_event(evt);
+ LEAVE;
+ return;
+ }
+
+ ibmvfc_set_host_action(vhost, IBMVFC_HOST_ACTION_QUERY);
+ wake_up(&vhost->work_wait_q);
+
+ LEAVE;
+}
+
+static void ibmvfc_fabric_login(struct ibmvfc_host *vhost)
+{
+ struct ibmvfc_fabric_login *mad;
+ struct ibmvfc_event *evt;
+ int level = IBMVFC_DEFAULT_LOG_LEVEL;
+
+ if (vhost->scsi_scrqs.protocol != IBMVFC_PROTO_SCSI) {
+ ibmvfc_log(vhost, level, "Fabric Login failed: unknown protocol\n");
+ ibmvfc_hard_reset_host(vhost);
+ return;
+ }
+
+ evt = ibmvfc_get_reserved_event(&vhost->crq);
+ if (!evt) {
+ ibmvfc_log(vhost, level, "Fabric Login failed: no available events\n");
+ ibmvfc_hard_reset_host(vhost);
+ return;
+ }
+
+ ibmvfc_init_event(evt, ibmvfc_fabric_login_done, IBMVFC_MAD_FORMAT);
+ mad = &evt->iu.fabric_login;
+ memset(mad, 0, sizeof(*mad));
+ mad->common.opcode = cpu_to_be32(IBMVFC_FABRIC_LOGIN);
+ mad->common.version = cpu_to_be32(1);
+ mad->common.length = cpu_to_be16(sizeof(*mad));
+
+ ibmvfc_set_host_action(vhost, IBMVFC_HOST_ACTION_INIT_WAIT);
+
+ if (ibmvfc_send_event(evt, vhost, default_timeout))
+ ibmvfc_link_down(vhost, IBMVFC_LINK_DOWN);
+}
+
static void ibmvfc_channel_setup_done(struct ibmvfc_event *evt)
{
struct ibmvfc_host *vhost = evt->vhost;
@@ -5311,8 +5393,12 @@ static void ibmvfc_channel_setup_done(struct ibmvfc_event *evt)
return;
}
- ibmvfc_set_host_action(vhost, IBMVFC_HOST_ACTION_QUERY);
- wake_up(&vhost->work_wait_q);
+ if (ibmvfc_check_caps(vhost, IBMVFC_SUPPORT_SCSI)) {
+ ibmvfc_fabric_login(vhost);
+ } else {
+ ibmvfc_set_host_action(vhost, IBMVFC_HOST_ACTION_QUERY);
+ wake_up(&vhost->work_wait_q);
+ }
}
static void ibmvfc_channel_setup(struct ibmvfc_host *vhost)
@@ -5503,9 +5589,11 @@ static void ibmvfc_npiv_login_done(struct ibmvfc_event *evt)
vhost->host->can_queue = be32_to_cpu(rsp->max_cmds) - IBMVFC_NUM_INTERNAL_REQ;
vhost->host->max_sectors = npiv_max_sectors;
- if (ibmvfc_check_caps(vhost, IBMVFC_CAN_SUPPORT_CHANNELS) && vhost->do_enquiry) {
+ if (ibmvfc_check_caps(vhost, IBMVFC_CAN_SUPPORT_CHANNELS) && vhost->do_enquiry)
ibmvfc_channel_enquiry(vhost);
- } else {
+ else if (ibmvfc_check_caps(vhost, IBMVFC_SUPPORT_SCSI))
+ ibmvfc_fabric_login(vhost);
+ else {
vhost->do_enquiry = 0;
ibmvfc_set_host_action(vhost, IBMVFC_HOST_ACTION_QUERY);
wake_up(&vhost->work_wait_q);
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.h b/drivers/scsi/ibmvscsi/ibmvfc.h
index 526632cb7237..adfd67e85af8 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.h
+++ b/drivers/scsi/ibmvscsi/ibmvfc.h
@@ -138,6 +138,7 @@ enum ibmvfc_mad_types {
IBMVFC_CHANNEL_ENQUIRY = 0x1000,
IBMVFC_CHANNEL_SETUP = 0x2000,
IBMVFC_CONNECTION_INFO = 0x4000,
+ IBMVFC_FABRIC_LOGIN = 0x8000,
};
struct ibmvfc_mad_common {
@@ -180,6 +181,7 @@ struct ibmvfc_npiv_login {
#define IBMVFC_CAN_HANDLE_FPIN 0x004
#define IBMVFC_CAN_USE_MAD_VERSION 0x008
#define IBMVFC_CAN_SEND_VF_WWPN 0x010
+#define IBMVFC_YES_SCSI 0x040
#define IBMVFC_CAN_USE_NOOP_CMD 0x200
__be64 node_name;
struct srp_direct_buf async;
@@ -227,6 +229,7 @@ struct ibmvfc_npiv_login_resp {
#define IBMVFC_MAD_VERSION_CAP 0x0020
#define IBMVFC_HANDLE_VF_WWPN 0x0040
#define IBMVFC_CAN_SUPPORT_CHANNELS 0x0080
+#define IBMVFC_SUPPORT_SCSI 0x0200
#define IBMVFC_SUPPORT_NOOP_CMD 0x1000
__be32 max_cmds;
__be32 scsi_id_sz;
@@ -590,6 +593,19 @@ struct ibmvfc_connection_info {
__be64 reserved[16];
} __packed __aligned(8);
+struct ibmvfc_fabric_login {
+ struct ibmvfc_mad_common common;
+ __be64 flags;
+#define IBMVFC_STRIP_MERGE 0x1
+#define IBMVFC_LINK_COMMANDS 0x2
+ __be64 capabilities;
+ __be64 nport_id;
+ __be16 status;
+ __be16 error;
+ __be32 pad;
+ __be64 reserved[16];
+} __packed __aligned(8);
+
struct ibmvfc_trace_start_entry {
u32 xfer_len;
} __packed;
@@ -715,6 +731,7 @@ union ibmvfc_iu {
struct ibmvfc_channel_enquiry channel_enquiry;
struct ibmvfc_channel_setup_mad channel_setup;
struct ibmvfc_connection_info connection_info;
+ struct ibmvfc_fabric_login fabric_login;
} __packed __aligned(8);
enum ibmvfc_target_action {
--
2.55.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v4 4/8] ibmvfc: define asynchronous sub-queue
2026-07-10 19:16 [PATCH v4 0/8] ibmvfc: make ibmvfc support FPIN messages Dave Marquardt via B4 Relay
` (2 preceding siblings ...)
2026-07-10 19:16 ` [PATCH v4 3/8] ibmvfc: make ibmvfc login to fabric Dave Marquardt via B4 Relay
@ 2026-07-10 19:16 ` Dave Marquardt via B4 Relay
2026-07-10 19:40 ` sashiko-bot
2026-07-10 19:16 ` [PATCH v4 5/8] ibmvfc: allocate " Dave Marquardt via B4 Relay
` (3 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Dave Marquardt via B4 Relay @ 2026-07-10 19:16 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
Tyrel Datwyler
Cc: linux-kernel, linux-scsi, linuxppc-dev, Brian King, Greg Joyce,
Kyle Mahlkuch, Dave Marquardt
From: Dave Marquardt <davemarq@linux.ibm.com>
Define data structures for asynchronous sub-queue support required for
full and extended FPIN functionality.
Add ibmvfc_async_subq structure to represent async events received via
the sub-queue, including FPIN status, link state, event type, and WWPN
information.
Update ibmvfc_channel_setup structure to include async_subq_handle field
and reduce IBMVFC_MAX_CHANNELS from 502 to 501 to accommodate the async
sub-queue. Add async_scrq pointer to ibmvfc_channels structure.
Add capability flags IBMVFC_USE_ASYNC_SUBQ and IBMVFC_SUPPORT_ASYNC_SUBQ
for negotiating async sub-queue support with VIOS during login.
Signed-off-by: Dave Marquardt <davemarq@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.h | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.h b/drivers/scsi/ibmvscsi/ibmvfc.h
index adfd67e85af8..f38dfae9924c 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.h
+++ b/drivers/scsi/ibmvscsi/ibmvfc.h
@@ -182,6 +182,7 @@ struct ibmvfc_npiv_login {
#define IBMVFC_CAN_USE_MAD_VERSION 0x008
#define IBMVFC_CAN_SEND_VF_WWPN 0x010
#define IBMVFC_YES_SCSI 0x040
+#define IBMVFC_USE_ASYNC_SUBQ 0x100
#define IBMVFC_CAN_USE_NOOP_CMD 0x200
__be64 node_name;
struct srp_direct_buf async;
@@ -230,6 +231,7 @@ struct ibmvfc_npiv_login_resp {
#define IBMVFC_HANDLE_VF_WWPN 0x0040
#define IBMVFC_CAN_SUPPORT_CHANNELS 0x0080
#define IBMVFC_SUPPORT_SCSI 0x0200
+#define IBMVFC_SUPPORT_ASYNC_SUBQ 0x0800
#define IBMVFC_SUPPORT_NOOP_CMD 0x1000
__be32 max_cmds;
__be32 scsi_id_sz;
@@ -564,7 +566,7 @@ struct ibmvfc_channel_setup_mad {
struct srp_direct_buf buffer;
} __packed __aligned(8);
-#define IBMVFC_MAX_CHANNELS 502
+#define IBMVFC_MAX_CHANNELS 501
struct ibmvfc_channel_setup {
__be32 flags;
@@ -579,6 +581,7 @@ struct ibmvfc_channel_setup {
struct srp_direct_buf buffer;
__be64 reserved2[5];
__be64 channel_handles[IBMVFC_MAX_CHANNELS];
+ __be64 async_subq_handle;
} __packed __aligned(8);
struct ibmvfc_connection_info {
@@ -715,6 +718,25 @@ struct ibmvfc_async_work {
struct work_struct async_work_s;
};
+struct ibmvfc_async_subq {
+ volatile u8 valid;
+#define IBMVFC_ASYNC_ID_IS_ASSOC_ID 0x01
+#define IBMVFC_FC_EEH 0x04
+#define IBMVFC_FC_FW_UPDATE 0x08
+#define IBMVFC_FC_FW_DUMP 0x10
+ u8 flags;
+ u8 link_state;
+ u8 fpin_status;
+ __be16 event;
+ __be16 pad;
+ volatile __be64 wwpn;
+ volatile __be64 nport_id;
+ union {
+ __be64 node_name;
+ __be64 assoc_id;
+ } id;
+} __packed __aligned(8);
+
union ibmvfc_iu {
struct ibmvfc_mad_common mad_common;
struct ibmvfc_npiv_login_mad npiv_login;
@@ -854,6 +876,7 @@ struct ibmvfc_queue {
struct ibmvfc_channels {
struct ibmvfc_queue *scrqs;
+ struct ibmvfc_queue *async_scrq;
enum ibmvfc_protocol protocol;
unsigned int active_queues;
unsigned int desired_queues;
--
2.55.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v4 5/8] ibmvfc: allocate asynchronous sub-queue
2026-07-10 19:16 [PATCH v4 0/8] ibmvfc: make ibmvfc support FPIN messages Dave Marquardt via B4 Relay
` (3 preceding siblings ...)
2026-07-10 19:16 ` [PATCH v4 4/8] ibmvfc: define asynchronous sub-queue Dave Marquardt via B4 Relay
@ 2026-07-10 19:16 ` Dave Marquardt via B4 Relay
2026-07-10 19:36 ` sashiko-bot
2026-07-10 19:16 ` [PATCH v4 6/8] ibmvfc: extend async event handlers to handle async sub queue events Dave Marquardt via B4 Relay
` (2 subsequent siblings)
7 siblings, 1 reply; 17+ messages in thread
From: Dave Marquardt via B4 Relay @ 2026-07-10 19:16 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
Tyrel Datwyler
Cc: linux-kernel, linux-scsi, linuxppc-dev, Brian King, Greg Joyce,
Kyle Mahlkuch, Dave Marquardt
From: Dave Marquardt <davemarq@linux.ibm.com>
Allocate and initialize the asynchronous sub-queue required for receiving
full and extended FPIN events from VIOS.
Modify ibmvfc_alloc_channels() to allocate async_scrq using
ibmvfc_alloc_queue() with IBMVFC_SUB_CRQ_FMT format. Update error
handling to properly clean up async_scrq on allocation failures.
Update ibmvfc_channel_setup() to pass async_subq_handle to VIOS during
channel setup, and ibmvfc_channel_setup_done() to store the VIOS cookie
for the async sub-queue.
Modify ibmvfc_release_channels() to free async_scrq resources during
cleanup.
Signed-off-by: Dave Marquardt <davemarq@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 36 +++++++++++++++++++++++++++++++++---
1 file changed, 33 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index c95e78d729ed..586847ff3336 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -5375,6 +5375,9 @@ static void ibmvfc_channel_setup_done(struct ibmvfc_event *evt)
for (i = 0; i < active_queues; i++)
scrqs->scrqs[i].vios_cookie =
be64_to_cpu(setup->channel_handles[i]);
+ if (scrqs->async_scrq)
+ scrqs->async_scrq->vios_cookie =
+ be64_to_cpu(setup->async_subq_handle);
ibmvfc_dbg(vhost, "Using %u channels\n",
vhost->scsi_scrqs.active_queues);
@@ -5425,6 +5428,7 @@ static void ibmvfc_channel_setup(struct ibmvfc_host *vhost)
setup_buf->num_scsi_subq_channels = cpu_to_be32(num_channels);
for (i = 0; i < num_channels; i++)
setup_buf->channel_handles[i] = cpu_to_be64(scrqs->scrqs[i].cookie);
+ setup_buf->async_subq_handle = cpu_to_be64(scrqs->async_scrq->cookie);
}
ibmvfc_init_event(evt, ibmvfc_channel_setup_done, IBMVFC_MAD_FORMAT);
@@ -6392,6 +6396,17 @@ static int ibmvfc_alloc_channels(struct ibmvfc_host *vhost,
if (!channels->scrqs)
return -ENOMEM;
+ channels->async_scrq = kzalloc_obj(*channels->async_scrq, GFP_KERNEL);
+ if (!channels->async_scrq) {
+ rc = -ENOMEM;
+ goto free_scrqs;
+ }
+
+ rc = ibmvfc_alloc_queue(vhost, channels->async_scrq,
+ IBMVFC_SUB_CRQ_FMT);
+ if (rc)
+ goto free_async;
+
for (i = 0; i < channels->max_queues; i++) {
scrq = &channels->scrqs[i];
rc = ibmvfc_alloc_queue(vhost, scrq, IBMVFC_SUB_CRQ_FMT);
@@ -6400,13 +6415,21 @@ static int ibmvfc_alloc_channels(struct ibmvfc_host *vhost,
scrq = &channels->scrqs[j - 1];
ibmvfc_free_queue(vhost, scrq);
}
- kfree(channels->scrqs);
- channels->scrqs = NULL;
+ ibmvfc_free_queue(vhost, channels->async_scrq);
channels->active_queues = 0;
- return rc;
+ goto free_async;
}
}
+ return rc;
+
+free_async:
+ kfree(channels->async_scrq);
+ channels->async_scrq = NULL;
+free_scrqs:
+ kfree(channels->scrqs);
+ channels->scrqs = NULL;
+
return rc;
}
@@ -6441,8 +6464,15 @@ static void ibmvfc_release_channels(struct ibmvfc_host *vhost,
kfree(channels->scrqs);
channels->scrqs = NULL;
+
channels->active_queues = 0;
}
+
+ if (channels->async_scrq) {
+ ibmvfc_free_queue(vhost, channels->async_scrq);
+ kfree(channels->async_scrq);
+ channels->async_scrq = NULL;
+ }
}
static void ibmvfc_release_sub_crqs(struct ibmvfc_host *vhost)
--
2.55.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v4 6/8] ibmvfc: extend async event handlers to handle async sub queue events
2026-07-10 19:16 [PATCH v4 0/8] ibmvfc: make ibmvfc support FPIN messages Dave Marquardt via B4 Relay
` (4 preceding siblings ...)
2026-07-10 19:16 ` [PATCH v4 5/8] ibmvfc: allocate " Dave Marquardt via B4 Relay
@ 2026-07-10 19:16 ` Dave Marquardt via B4 Relay
2026-07-10 19:37 ` sashiko-bot
2026-07-10 19:16 ` [PATCH v4 7/8] ibmvfc: register and use asynchronous sub-queue for events Dave Marquardt via B4 Relay
2026-07-10 19:16 ` [PATCH v4 8/8] ibmvfc: handle extended FPIN events Dave Marquardt via B4 Relay
7 siblings, 1 reply; 17+ messages in thread
From: Dave Marquardt via B4 Relay @ 2026-07-10 19:16 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
Tyrel Datwyler
Cc: linux-kernel, linux-scsi, linuxppc-dev, Brian King, Greg Joyce,
Kyle Mahlkuch, Dave Marquardt
From: Dave Marquardt <davemarq@linux.ibm.com>
Refactor async event handling to support both traditional async CRQs and
new asynchronous sub-queue CRQs.
Modify ibmvfc_handle_async() to accept events from either source and
update ibmvfc_process_async_work() to handle both ibmvfc_async_crq and
ibmvfc_async_subq structures. Add is_subq flag to ibmvfc_async_work to
distinguish between event sources.
Add ibmvfc_full_fpin_to_desc() to convert full FPIN messages from async
sub-queue format to fc_els_fpin structures. Update FPIN processing logic
to extract WWPN, node_name, and scsi_id from the appropriate structure
based on event source.
Update KUnit tests to reflect the new async event handling interface.
Signed-off-by: Dave Marquardt <davemarq@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 166 +++++++++++++++++++++++++++--------
drivers/scsi/ibmvscsi/ibmvfc.h | 17 ++--
drivers/scsi/ibmvscsi/ibmvfc_kunit.c | 10 +--
3 files changed, 143 insertions(+), 50 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index 586847ff3336..ee56f13f1a97 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -3349,6 +3349,27 @@ ibmvfc_basic_fpin_to_desc(struct ibmvfc_async_crq *crq, u64 wwpn)
cpu_to_be32(1));
}
+/**
+ * ibmvfc_full_fpin_to_desc(): allocate and populate a struct fc_els_fpin struct
+ * containing a descriptor.
+ * @ibmvfc_fpin: Pointer to async subq FPIN data
+ *
+ * Allocate a struct fc_els_fpin containing a descriptor and populate
+ * based on data from *ibmvfc_fpin.
+ *
+ * Return:
+ * NULL - unable to allocate structure
+ * non-NULL - pointer to populated struct fc_els_fpin
+ */
+static struct fc_els_fpin *
+ibmvfc_full_fpin_to_desc(struct ibmvfc_async_subq *ibmvfc_fpin)
+{
+ return ibmvfc_common_fpin_to_desc(ibmvfc_fpin->fpin_status, ibmvfc_fpin->wwpn,
+ cpu_to_be16(0), cpu_to_be16(0),
+ cpu_to_be32(IBMVFC_FPIN_DEFAULT_EVENT_THRESHOLD),
+ cpu_to_be32(1));
+}
+
/**
* ibmvfc_process_async_work - Process IBMVFC_AE_FPIN async CRQ from work queue
* @work: pointer to work_struct
@@ -3356,73 +3377,131 @@ ibmvfc_basic_fpin_to_desc(struct ibmvfc_async_crq *crq, u64 wwpn)
static void ibmvfc_process_async_work(struct work_struct *work)
{
struct ibmvfc_target *tgt, *next;
+ struct ibmvfc_async_subq *subq = NULL;
struct ibmvfc_async_work *aw;
- struct ibmvfc_async_crq *crq;
+ struct ibmvfc_async_crq *crq = NULL;
struct ibmvfc_host *vhost;
- struct fc_els_fpin *fpin;
+ struct fc_els_fpin *fpin = NULL;
unsigned long flags;
+ __be64 node_name;
+ __be64 scsi_id;
+ bool is_subq;
+ __be64 wwpn;
aw = container_of(work, struct ibmvfc_async_work, async_work_s);
- crq = &aw->crq;
vhost = aw->vhost;
+ is_subq = aw->is_subq;
+ if (is_subq)
+ subq = &aw->crq.subq;
+ else
+ crq = &aw->crq.async_crq;
- if (!crq->scsi_id && !crq->wwpn && !crq->node_name)
- goto end;
+ if (crq) {
+ wwpn = crq->wwpn;
+ node_name = crq->node_name;
+ scsi_id = crq->scsi_id;
+ } else {
+ wwpn = subq->wwpn;
+ node_name = subq->id.node_name;
+ scsi_id = 0;
+ }
+
+ if (!scsi_id && !wwpn && !node_name)
+ goto free;
spin_lock_irqsave(vhost->host->host_lock, flags);
list_for_each_entry_safe(tgt, next, &vhost->targets, queue) {
- if (crq->scsi_id && cpu_to_be64(tgt->scsi_id) != crq->scsi_id)
+ if (scsi_id && cpu_to_be64(tgt->scsi_id) != scsi_id)
continue;
- if (crq->wwpn && cpu_to_be64(tgt->ids.port_name) != crq->wwpn)
+ if (wwpn && cpu_to_be64(tgt->ids.port_name) != wwpn)
continue;
- if (crq->node_name && cpu_to_be64(tgt->ids.node_name) != crq->node_name)
+ if (node_name && cpu_to_be64(tgt->ids.node_name) != node_name)
continue;
if (!tgt->rport)
continue;
break;
}
- spin_unlock_irqrestore(vhost->host->host_lock, flags);
+ if (!list_entry_is_head(tgt, &vhost->targets, queue)) {
+ kref_get(&tgt->kref);
+ spin_unlock_irqrestore(vhost->host->host_lock, flags);
+ } else {
+ spin_unlock_irqrestore(vhost->host->host_lock, flags);
+ dev_err_ratelimited(vhost->dev, "Invalid target for FPIN\n");
+ goto free;
+ }
- if (list_entry_is_head(tgt, &vhost->targets, queue) || !tgt->rport) {
+ if (!tgt->rport) {
dev_err_ratelimited(vhost->dev, "Invalid target for FPIN\n");
goto end;
}
- fpin = ibmvfc_basic_fpin_to_desc(crq, tgt->wwpn);
+ if (crq)
+ fpin = ibmvfc_basic_fpin_to_desc(crq, tgt->wwpn);
+ else
+ fpin = ibmvfc_full_fpin_to_desc(subq);
+
if (fpin) {
fc_host_fpin_rcv(tgt->vhost->host,
sizeof(*fpin) + be32_to_cpu(fpin->desc_len),
(char *)fpin, 0);
kfree(fpin);
} else
- dev_err_ratelimited(vhost->dev,
- "FPIN event %u received, unable to process\n",
- crq->fpin_status);
+ dev_err_ratelimited(vhost->dev, "FPIN event received, unable to process\n");
+
end:
+ kref_put(&tgt->kref, ibmvfc_release_tgt);
+ free:
kfree(aw);
}
/**
* ibmvfc_handle_async - Handle an async event from the adapter
- * @crq: crq to process
+ * @crq: ibmvfc_async_crq or ibmvfc_async_subq
* @vhost: ibmvfc host struct
+ * @is_subq: indicates whether the crq points to a struct ibmvfc_async_subq
*
**/
-VISIBLE_IF_KUNIT void ibmvfc_handle_async(struct ibmvfc_async_crq *crq,
- struct ibmvfc_host *vhost)
+VISIBLE_IF_KUNIT void ibmvfc_handle_async(void *crq,
+ struct ibmvfc_host *vhost,
+ bool is_subq)
{
- const struct ibmvfc_async_desc *desc = ibmvfc_get_ae_desc(be64_to_cpu(crq->event));
+ struct ibmvfc_async_crq *async_crq = NULL;
+ struct ibmvfc_async_subq *subq = NULL;
+ const struct ibmvfc_async_desc *desc;
+ struct ibmvfc_target *tgt, *next;
struct ibmvfc_async_work *aw;
- struct ibmvfc_target *tgt;
-
- ibmvfc_log(vhost, desc->log_level, "%s event received. scsi_id: %llx, wwpn: %llx,"
- " node_name: %llx%s\n", desc->desc, be64_to_cpu(crq->scsi_id),
- be64_to_cpu(crq->wwpn), be64_to_cpu(crq->node_name),
- ibmvfc_get_link_state(crq->link_state));
-
- switch (be64_to_cpu(crq->event)) {
+ __be64 node_name;
+ __be64 scsi_id;
+ u8 link_state;
+ __be64 wwpn;
+ u64 event;
+
+ if (is_subq) {
+ subq = crq;
+ event = be16_to_cpu(subq->event);
+ link_state = subq->link_state;
+ scsi_id = 0;
+ wwpn = subq->wwpn;
+ node_name = subq->id.node_name;
+ } else {
+ async_crq = crq;
+ event = be64_to_cpu(async_crq->event);
+ link_state = async_crq->link_state;
+ scsi_id = async_crq->scsi_id;
+ wwpn = async_crq->wwpn;
+ node_name = async_crq->node_name;
+ }
+
+ desc = ibmvfc_get_ae_desc(event);
+ ibmvfc_log(vhost, desc->log_level,
+ "%s event received. scsi_id: %llx, wwpn: %llx, node_name: %llx, event %llx%s\n",
+ desc->desc, be64_to_cpu(scsi_id),
+ be64_to_cpu(wwpn), be64_to_cpu(node_name), event,
+ ibmvfc_get_link_state(link_state));
+
+ switch (event) {
case IBMVFC_AE_RESUME:
- switch (crq->link_state) {
+ switch (link_state) {
case IBMVFC_AE_LS_LINK_DOWN:
ibmvfc_link_down(vhost, IBMVFC_LINK_DOWN);
break;
@@ -3460,18 +3539,18 @@ VISIBLE_IF_KUNIT void ibmvfc_handle_async(struct ibmvfc_async_crq *crq,
case IBMVFC_AE_ELS_LOGO:
case IBMVFC_AE_ELS_PRLO:
case IBMVFC_AE_ELS_PLOGI:
- list_for_each_entry(tgt, &vhost->targets, queue) {
- if (!crq->scsi_id && !crq->wwpn && !crq->node_name)
+ list_for_each_entry_safe(tgt, next, &vhost->targets, queue) {
+ if (!scsi_id && !wwpn && !node_name)
break;
- if (crq->scsi_id && cpu_to_be64(tgt->scsi_id) != crq->scsi_id)
+ if (scsi_id && cpu_to_be64(tgt->scsi_id) != scsi_id)
continue;
- if (crq->wwpn && cpu_to_be64(tgt->ids.port_name) != crq->wwpn)
+ if (wwpn && cpu_to_be64(tgt->ids.port_name) != wwpn)
continue;
- if (crq->node_name && cpu_to_be64(tgt->ids.node_name) != crq->node_name)
+ if (node_name && cpu_to_be64(tgt->ids.node_name) != node_name)
continue;
- if (tgt->need_login && be64_to_cpu(crq->event) == IBMVFC_AE_ELS_LOGO)
+ if (tgt->need_login && event == IBMVFC_AE_ELS_LOGO)
tgt->logo_rcvd = 1;
- if (!tgt->need_login || be64_to_cpu(crq->event) == IBMVFC_AE_ELS_PLOGI) {
+ if (!tgt->need_login || event == IBMVFC_AE_ELS_PLOGI) {
ibmvfc_del_tgt(tgt);
ibmvfc_reinit_host(vhost);
}
@@ -3492,16 +3571,27 @@ VISIBLE_IF_KUNIT void ibmvfc_handle_async(struct ibmvfc_async_crq *crq,
if (aw) {
INIT_WORK(&aw->async_work_s, ibmvfc_process_async_work);
aw->vhost = vhost;
- aw->crq = *crq;
+ aw->is_subq = is_subq;
+ if (is_subq)
+ aw->crq.subq = *subq;
+ else
+ aw->crq.async_crq = *async_crq;
queue_work(vhost->fpin_workq, &aw->async_work_s);
} else
dev_err_ratelimited(vhost->dev,
"can't offload async CRQ to work queue\n");
break;
default:
- dev_err(vhost->dev, "Unknown async event received: %lld\n", crq->event);
+ dev_err(vhost->dev, "Unknown async event received: %llu\n", event);
break;
}
+
+ rmb();
+ if (is_subq)
+ subq->valid = 0;
+ else
+ async_crq->valid = 0;
+ wmb();
}
EXPORT_SYMBOL_IF_KUNIT(ibmvfc_handle_async);
@@ -4040,7 +4130,7 @@ static void ibmvfc_tasklet(void *data)
while (!done) {
/* Pull all the valid messages off the async CRQ */
while ((async = ibmvfc_next_async_crq(vhost)) != NULL) {
- ibmvfc_handle_async(async, vhost);
+ ibmvfc_handle_async(async, vhost, false);
}
/* Pull all the valid messages off the CRQ */
@@ -4053,7 +4143,7 @@ static void ibmvfc_tasklet(void *data)
vio_enable_interrupts(vdev);
if ((async = ibmvfc_next_async_crq(vhost)) != NULL) {
vio_disable_interrupts(vdev);
- ibmvfc_handle_async(async, vhost);
+ ibmvfc_handle_async(async, vhost, false);
} else if ((crq = ibmvfc_next_crq(vhost)) != NULL) {
vio_disable_interrupts(vdev);
ibmvfc_handle_crq(crq, vhost, &evt_doneq);
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.h b/drivers/scsi/ibmvscsi/ibmvfc.h
index f38dfae9924c..bbf19220af70 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.h
+++ b/drivers/scsi/ibmvscsi/ibmvfc.h
@@ -712,12 +712,6 @@ struct ibmvfc_async_crq {
__be64 reserved;
} __packed __aligned(8);
-struct ibmvfc_async_work {
- struct ibmvfc_host *vhost;
- struct ibmvfc_async_crq crq;
- struct work_struct async_work_s;
-};
-
struct ibmvfc_async_subq {
volatile u8 valid;
#define IBMVFC_ASYNC_ID_IS_ASSOC_ID 0x01
@@ -737,6 +731,15 @@ struct ibmvfc_async_subq {
} id;
} __packed __aligned(8);
+struct ibmvfc_async_work {
+ struct ibmvfc_host *vhost;
+ bool is_subq;
+ union {
+ struct ibmvfc_async_crq async_crq;
+ struct ibmvfc_async_subq subq;
+ } crq;
+ struct work_struct async_work_s;
+};
union ibmvfc_iu {
struct ibmvfc_mad_common mad_common;
struct ibmvfc_npiv_login_mad npiv_login;
@@ -1008,7 +1011,7 @@ struct ibmvfc_host {
#endif
#ifdef VISIBLE_IF_KUNIT
-VISIBLE_IF_KUNIT void ibmvfc_handle_async(struct ibmvfc_async_crq *crq, struct ibmvfc_host *vhost);
+VISIBLE_IF_KUNIT void ibmvfc_handle_async(void *crq, struct ibmvfc_host *vhost, bool is_subq);
VISIBLE_IF_KUNIT struct list_head *ibmvfc_get_headp(void);
#endif
diff --git a/drivers/scsi/ibmvscsi/ibmvfc_kunit.c b/drivers/scsi/ibmvscsi/ibmvfc_kunit.c
index 1c90318b6811..86c1f6daca58 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc_kunit.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc_kunit.c
@@ -45,7 +45,7 @@ static void ibmvfc_async_fpin_test(struct kunit *test)
fc_host = shost_to_fc_host(vhost->host);
pre[IBMVFC_AE_FPIN_LINK_CONGESTED] = READ_ONCE(fc_host->fpin_stats.cn_device_specific);
- pre[IBMVFC_AE_FPIN_PORT_CONGESTED] = READ_ONCE(tgt->rport->fpin_stats.cn);
+ pre[IBMVFC_AE_FPIN_PORT_CONGESTED] = READ_ONCE(tgt->rport->fpin_stats.cn_device_specific);
pre[IBMVFC_AE_FPIN_PORT_CLEARED] = READ_ONCE(tgt->rport->fpin_stats.cn_clear);
pre[IBMVFC_AE_FPIN_PORT_DEGRADED] = READ_ONCE(tgt->rport->fpin_stats.li_failure_unknown);
pre[IBMVFC_AE_FPIN_CONGESTION_CLEARED] = READ_ONCE(fc_host->fpin_stats.cn_clear);
@@ -58,7 +58,8 @@ static void ibmvfc_async_fpin_test(struct kunit *test)
crq[fs].scsi_id = cpu_to_be64(tgt->scsi_id);
crq[fs].wwpn = cpu_to_be64(tgt->wwpn);
crq[fs].node_name = cpu_to_be64(tgt->ids.node_name);
- ibmvfc_handle_async(&crq[fs], vhost);
+ ibmvfc_handle_async(&crq[fs], vhost, false);
+ msleep(1U);
}
msleep(500U);
@@ -94,9 +95,8 @@ static void ibmvfc_async_fpin_test(struct kunit *test)
crq[0].scsi_id = cpu_to_be64(tgt->scsi_id);
crq[0].wwpn = cpu_to_be64(tgt->wwpn);
crq[0].node_name = cpu_to_be64(tgt->ids.node_name);
- ibmvfc_handle_async(&crq[0], vhost);
-
- msleep(500U);
+ ibmvfc_handle_async(&crq[0], vhost, false);
+ msleep(1U);
post[IBMVFC_AE_FPIN_LINK_CONGESTED] = READ_ONCE(fc_host->fpin_stats.cn_device_specific);
post[IBMVFC_AE_FPIN_PORT_CONGESTED] = READ_ONCE(tgt->rport->fpin_stats.cn);
--
2.55.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v4 7/8] ibmvfc: register and use asynchronous sub-queue for events
2026-07-10 19:16 [PATCH v4 0/8] ibmvfc: make ibmvfc support FPIN messages Dave Marquardt via B4 Relay
` (5 preceding siblings ...)
2026-07-10 19:16 ` [PATCH v4 6/8] ibmvfc: extend async event handlers to handle async sub queue events Dave Marquardt via B4 Relay
@ 2026-07-10 19:16 ` Dave Marquardt via B4 Relay
2026-07-10 19:32 ` sashiko-bot
2026-07-10 19:16 ` [PATCH v4 8/8] ibmvfc: handle extended FPIN events Dave Marquardt via B4 Relay
7 siblings, 1 reply; 17+ messages in thread
From: Dave Marquardt via B4 Relay @ 2026-07-10 19:16 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
Tyrel Datwyler
Cc: linux-kernel, linux-scsi, linuxppc-dev, Brian King, Greg Joyce,
Kyle Mahlkuch, Dave Marquardt
From: Dave Marquardt <davemarq@linux.ibm.com>
Complete async sub-queue integration by setting up interrupt handling,
registering the queue as a channel, and enabling its use during NPIV
login.
Add ibmvfc_interrupt_async_subq() interrupt handler and
ibmvfc_drain_async_subq() to process events from the async sub-queue.
Refactor ibmvfc_register_channel() into ibmvfc_register_channel_common()
to support both regular sub-CRQs and the async sub-queue with different
interrupt handlers.
Update ibmvfc_set_login_info() to set IBMVFC_CAN_USE_CHANNELS,
IBMVFC_YES_SCSI, IBMVFC_USE_ASYNC_SUBQ, and IBMVFC_CAN_HANDLE_FPIN
capability bits when channels are enabled, informing VIOS that the client
supports async sub-queue and FPIN handling.
Register async_scrq during channel initialization and unregister during
cleanup.
---
drivers/scsi/ibmvscsi/ibmvfc.c | 146 ++++++++++++++++++++++++++++++++++-------
1 file changed, 124 insertions(+), 22 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index ee56f13f1a97..eb786ac24274 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -1517,7 +1517,9 @@ static void ibmvfc_set_login_info(struct ibmvfc_host *vhost)
IBMVFC_CAN_USE_NOOP_CMD);
if (vhost->mq_enabled || vhost->using_channels)
- login_info->capabilities |= cpu_to_be64(IBMVFC_CAN_USE_CHANNELS);
+ login_info->capabilities |=
+ cpu_to_be64(IBMVFC_CAN_USE_CHANNELS | IBMVFC_YES_SCSI |
+ IBMVFC_USE_ASYNC_SUBQ | IBMVFC_CAN_HANDLE_FPIN);
login_info->async.va = cpu_to_be64(vhost->async_crq.msg_token);
login_info->async.len = cpu_to_be32(async_crq->size *
@@ -4243,6 +4245,52 @@ static struct ibmvfc_crq *ibmvfc_next_scrq(struct ibmvfc_queue *scrq)
return crq;
}
+static void ibmvfc_drain_async_subq(struct ibmvfc_queue *scrq)
+{
+ struct ibmvfc_host *vhost = scrq->vhost;
+ struct ibmvfc_crq *crq;
+ unsigned long flags;
+ int done = 0;
+
+ spin_lock_irqsave(vhost->host->host_lock, flags);
+ spin_lock(scrq->q_lock);
+ while (!done) {
+ while ((crq = ibmvfc_next_scrq(scrq)) != NULL) {
+ ibmvfc_handle_async(crq, scrq->vhost, true);
+ crq->valid = 0;
+ wmb(); /* complete write */
+ }
+
+ ibmvfc_toggle_scrq_irq(scrq, 1);
+ crq = ibmvfc_next_scrq(scrq);
+ if (crq != NULL) {
+ ibmvfc_toggle_scrq_irq(scrq, 0);
+ ibmvfc_handle_async(crq, scrq->vhost, true);
+ crq->valid = 0;
+ wmb(); /* complete write */
+ } else
+ done = 1;
+ }
+ spin_unlock(scrq->q_lock);
+ spin_unlock_irqrestore(vhost->host->host_lock, flags);
+}
+
+/**
+ * ibmvfc_interrupt_asyncq - Handle an async event from the adapter
+ * @irq: interrupt request
+ * @scrq_instance: async subq
+ *
+ **/
+static irqreturn_t ibmvfc_interrupt_async_subq(int irq, void *scrq_instance)
+{
+ struct ibmvfc_queue *scrq = (struct ibmvfc_queue *)scrq_instance;
+
+ ibmvfc_toggle_scrq_irq(scrq, 0);
+ ibmvfc_drain_async_subq(scrq);
+
+ return IRQ_HANDLED;
+}
+
static void ibmvfc_drain_sub_crq(struct ibmvfc_queue *scrq)
{
struct ibmvfc_crq *crq;
@@ -6340,14 +6388,29 @@ static int ibmvfc_init_crq(struct ibmvfc_host *vhost)
return retrc;
}
-static int ibmvfc_register_channel(struct ibmvfc_host *vhost,
- struct ibmvfc_channels *channels,
- int index)
+/**
+ * ibmvfc_register_channel_common - Register a sub-CRQ with the hypervisor
+ * @vhost: ibmvfc host struct
+ * @channels: ibmvfc channels struct
+ * @scrq: sub-CRQ to register
+ * @index: channel index (negative for async)
+ * @irq: interrupt handler for the sub-CRQ
+ *
+ * Return value:
+ * 0 on success / non-zero on failure
+ **/
+static int ibmvfc_register_channel_common(struct ibmvfc_host *vhost,
+ struct ibmvfc_channels *channels,
+ struct ibmvfc_queue *scrq,
+ int index,
+ irq_handler_t irq)
{
struct device *dev = vhost->dev;
struct vio_dev *vdev = to_vio_dev(dev);
- struct ibmvfc_queue *scrq = &channels->scrqs[index];
+ long hcall_rc;
int rc = -ENOMEM;
+ const char *name_suffix;
+ bool is_async = (index < 0);
ENTER;
@@ -6366,20 +6429,19 @@ static int ibmvfc_register_channel(struct ibmvfc_host *vhost,
if (!scrq->irq) {
rc = -EINVAL;
- dev_err(dev, "Error mapping sub-crq[%d] irq\n", index);
+ if (is_async)
+ dev_err(dev, "Error mapping sub-crq[%s] irq\n", "async");
+ else
+ dev_err(dev, "Error mapping sub-crq[%d] irq\n", index);
goto irq_failed;
}
switch (channels->protocol) {
case IBMVFC_PROTO_SCSI:
- snprintf(scrq->name, sizeof(scrq->name), "ibmvfc-%x-scsi%d",
- vdev->unit_address, index);
- scrq->handler = ibmvfc_interrupt_mq;
+ name_suffix = "scsi";
break;
case IBMVFC_PROTO_NVME:
- snprintf(scrq->name, sizeof(scrq->name), "ibmvfc-%x-nvmf%d",
- vdev->unit_address, index);
- scrq->handler = ibmvfc_interrupt_mq;
+ name_suffix = "nvmf";
break;
default:
dev_err(dev, "Unknown channel protocol (%d)\n",
@@ -6387,35 +6449,63 @@ static int ibmvfc_register_channel(struct ibmvfc_host *vhost,
goto irq_failed;
}
+ if (is_async) {
+ snprintf(scrq->name, sizeof(scrq->name), "ibmvfc-%x-%s%s",
+ vdev->unit_address, name_suffix, "async");
+ scrq->handler = irq;
+ } else {
+ snprintf(scrq->name, sizeof(scrq->name), "ibmvfc-%x-%s%d",
+ vdev->unit_address, name_suffix, index);
+ scrq->handler = irq ? irq : ibmvfc_interrupt_mq;
+ scrq->hwq_id = index;
+ }
+
rc = request_irq(scrq->irq, scrq->handler, 0, scrq->name, scrq);
if (rc) {
- dev_err(dev, "Couldn't register sub-crq[%d] irq\n", index);
+ if (is_async)
+ dev_err(dev, "Couldn't register sub-crq[%s] irq\n", "async");
+ else
+ dev_err(dev, "Couldn't register sub-crq[%d] irq\n", index);
irq_dispose_mapping(scrq->irq);
goto irq_failed;
}
- scrq->hwq_id = index;
-
LEAVE;
return 0;
irq_failed:
do {
- rc = plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address, scrq->cookie);
- } while (rc == H_BUSY || H_IS_LONG_BUSY(rc));
+ hcall_rc = plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address, scrq->cookie);
+ } while (hcall_rc == H_BUSY || H_IS_LONG_BUSY(hcall_rc));
reg_failed:
LEAVE;
return rc;
}
+static int ibmvfc_register_channel_async(struct ibmvfc_host *vhost,
+ struct ibmvfc_channels *channels,
+ struct ibmvfc_queue *scrq,
+ irq_handler_t irq)
+{
+ return ibmvfc_register_channel_common(vhost, channels, scrq, -1, irq);
+}
+
+static int ibmvfc_register_channel(struct ibmvfc_host *vhost,
+ struct ibmvfc_channels *channels,
+ int index)
+{
+ struct ibmvfc_queue *scrq = &channels->scrqs[index];
+
+ return ibmvfc_register_channel_common(vhost, channels, scrq, index, NULL);
+}
+
static void ibmvfc_deregister_channel(struct ibmvfc_host *vhost,
struct ibmvfc_channels *channels,
- int index)
+ struct ibmvfc_queue *scrq)
{
struct device *dev = vhost->dev;
struct vio_dev *vdev = to_vio_dev(dev);
- struct ibmvfc_queue *scrq = &channels->scrqs[index];
long rc;
ENTER;
@@ -6430,7 +6520,7 @@ static void ibmvfc_deregister_channel(struct ibmvfc_host *vhost,
} while (rc == H_BUSY || H_IS_LONG_BUSY(rc));
if (rc)
- dev_err(dev, "Failed to free sub-crq[%d]: rc=%ld\n", index, rc);
+ dev_err(dev, "Failed to free sub-crq[%s]: rc=%ld\n", scrq->name, rc);
/* Clean out the queue */
memset(scrq->msgs.crq, 0, PAGE_SIZE);
@@ -6448,10 +6538,21 @@ static void ibmvfc_reg_sub_crqs(struct ibmvfc_host *vhost,
if (!vhost->mq_enabled || !channels->scrqs)
return;
+ if (ibmvfc_register_channel_async(vhost, channels,
+ channels->async_scrq,
+ ibmvfc_interrupt_async_subq)) {
+ vhost->do_enquiry = 0;
+ return;
+ }
+
for (i = 0; i < channels->max_queues; i++) {
if (ibmvfc_register_channel(vhost, channels, i)) {
for (j = i; j > 0; j--)
- ibmvfc_deregister_channel(vhost, channels, j - 1);
+ ibmvfc_deregister_channel(
+ vhost, channels, &channels->scrqs[j - 1]);
+ ibmvfc_deregister_channel(vhost, channels,
+ channels->async_scrq);
+
vhost->do_enquiry = 0;
return;
}
@@ -6470,7 +6571,8 @@ static void ibmvfc_dereg_sub_crqs(struct ibmvfc_host *vhost,
return;
for (i = 0; i < channels->max_queues; i++)
- ibmvfc_deregister_channel(vhost, channels, i);
+ ibmvfc_deregister_channel(vhost, channels, &channels->scrqs[i]);
+ ibmvfc_deregister_channel(vhost, channels, channels->async_scrq);
LEAVE;
}
--
2.55.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [PATCH v4 8/8] ibmvfc: handle extended FPIN events
2026-07-10 19:16 [PATCH v4 0/8] ibmvfc: make ibmvfc support FPIN messages Dave Marquardt via B4 Relay
` (6 preceding siblings ...)
2026-07-10 19:16 ` [PATCH v4 7/8] ibmvfc: register and use asynchronous sub-queue for events Dave Marquardt via B4 Relay
@ 2026-07-10 19:16 ` Dave Marquardt via B4 Relay
2026-07-10 19:40 ` sashiko-bot
7 siblings, 1 reply; 17+ messages in thread
From: Dave Marquardt via B4 Relay @ 2026-07-10 19:16 UTC (permalink / raw)
To: James E.J. Bottomley, Martin K. Petersen, Madhavan Srinivasan,
Michael Ellerman, Nicholas Piggin, Christophe Leroy (CS GROUP),
Tyrel Datwyler
Cc: linux-kernel, linux-scsi, linuxppc-dev, Brian King, Greg Joyce,
Kyle Mahlkuch, Dave Marquardt
From: Dave Marquardt <davemarq@linux.ibm.com>
Implement support for extended FPIN messages received via the
asynchronous sub-queue, completing full FPIN functionality.
Extended FPIN messages provide more detailed information about fabric
events compared to basic FPIN messages, including specific event types,
modifiers, thresholds, and event counts.
Add ibmvfc_extended_fpin_to_desc() to convert extended FPIN messages from
async sub-queue format to fc_els_fpin structures with complete descriptor
information. Update ibmvfc_process_async_work() to handle extended FPIN
events from the async sub-queue.
Set IBMVFC_CAN_HANDLE_FPIN capability during login to inform VIOS that
the client can process extended FPIN messages.
Add comprehensive KUnit tests to validate extended FPIN event handling
and verify proper statistics updates for all FPIN event types.
Signed-off-by: Dave Marquardt <davemarq@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.c | 55 ++++++++++++++++-
drivers/scsi/ibmvscsi/ibmvfc.h | 31 ++++++++++
drivers/scsi/ibmvscsi/ibmvfc_kunit.c | 114 ++++++++++++++++++++++++++++++++++-
3 files changed, 194 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
index eb786ac24274..b131a4a3041b 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc.c
@@ -1519,7 +1519,8 @@ static void ibmvfc_set_login_info(struct ibmvfc_host *vhost)
if (vhost->mq_enabled || vhost->using_channels)
login_info->capabilities |=
cpu_to_be64(IBMVFC_CAN_USE_CHANNELS | IBMVFC_YES_SCSI |
- IBMVFC_USE_ASYNC_SUBQ | IBMVFC_CAN_HANDLE_FPIN);
+ IBMVFC_USE_ASYNC_SUBQ | IBMVFC_CAN_HANDLE_FPIN |
+ IBMVFC_CAN_HANDLE_FPIN_EXT);
login_info->async.va = cpu_to_be64(vhost->async_crq.msg_token);
login_info->async.len = cpu_to_be32(async_crq->size *
@@ -3372,12 +3373,48 @@ ibmvfc_full_fpin_to_desc(struct ibmvfc_async_subq *ibmvfc_fpin)
cpu_to_be32(1));
}
+/**
+ * ibmvfc_ext_fpin_to_desc(): allocate and populate a struct fc_els_fpin struct
+ * containing a descriptor.
+ * @ibmvfc_fpin: Pointer to async subq FPIN data
+ *
+ * Allocate a struct fc_els_fpin containing a descriptor and populate
+ * based on data from *ibmvfc_fpin.
+ *
+ * Return:
+ * NULL - unable to allocate structure
+ * non-NULL - pointer to populated struct fc_els_fpin
+ */
+static struct fc_els_fpin *
+ibmvfc_ext_fpin_to_desc(struct ibmvfc_async_subq_fpin *ibmvfc_fpin)
+{
+ u8 flags = ibmvfc_fpin->fpin_data.flags;
+ __be32 threshold = cpu_to_be32(IBMVFC_FPIN_DEFAULT_EVENT_THRESHOLD);
+ __be16 modifier = 0;
+ __be32 count = cpu_to_be32(1);
+ __be16 type = 0;
+
+ if (flags & IBMVFC_FPIN_EVENT_TYPE_VALID)
+ type = ibmvfc_fpin->fpin_data.event_type;
+ if (flags & IBMVFC_FPIN_MODIFIER_VALID)
+ modifier = ibmvfc_fpin->fpin_data.event_type_modifier;
+ if (flags & IBMVFC_FPIN_THRESHOLD_VALID)
+ threshold = ibmvfc_fpin->fpin_data.event_threshold;
+ if (flags & IBMVFC_FPIN_EVENT_COUNT_VALID)
+ count = ibmvfc_fpin->fpin_data.event_data.event_count;
+
+ return ibmvfc_common_fpin_to_desc(ibmvfc_fpin->fpin_status,
+ ibmvfc_fpin->wwpn, type,
+ modifier, threshold, count);
+}
+
/**
* ibmvfc_process_async_work - Process IBMVFC_AE_FPIN async CRQ from work queue
* @work: pointer to work_struct
*/
static void ibmvfc_process_async_work(struct work_struct *work)
{
+ struct ibmvfc_async_subq_fpin *sqfpin;
struct ibmvfc_target *tgt, *next;
struct ibmvfc_async_subq *subq = NULL;
struct ibmvfc_async_work *aw;
@@ -3439,8 +3476,20 @@ static void ibmvfc_process_async_work(struct work_struct *work)
if (crq)
fpin = ibmvfc_basic_fpin_to_desc(crq, tgt->wwpn);
- else
- fpin = ibmvfc_full_fpin_to_desc(subq);
+ else {
+ sqfpin = (struct ibmvfc_async_subq_fpin *)subq;
+ if ((subq->flags & IBMVFC_ASYNC_IS_FPIN_EXT) == 0) {
+ fpin = ibmvfc_full_fpin_to_desc(subq);
+ } else if (!(sqfpin->fpin_data.flags & IBMVFC_FPIN_EVENT_TYPE_VALID)) {
+ dev_err_ratelimited(vhost->dev,
+ "Invalid extended FPIN event received\n");
+ } else if (!ibmvfc_check_caps(vhost, IBMVFC_SUPPORT_FPIN_EXT)) {
+ dev_err_ratelimited(vhost->dev,
+ "Unexpected extended FPIN event received\n");
+ } else {
+ fpin = ibmvfc_ext_fpin_to_desc(sqfpin);
+ }
+ }
if (fpin) {
fc_host_fpin_rcv(tgt->vhost->host,
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.h b/drivers/scsi/ibmvscsi/ibmvfc.h
index bbf19220af70..d9ee270e0ef9 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.h
+++ b/drivers/scsi/ibmvscsi/ibmvfc.h
@@ -184,6 +184,7 @@ struct ibmvfc_npiv_login {
#define IBMVFC_YES_SCSI 0x040
#define IBMVFC_USE_ASYNC_SUBQ 0x100
#define IBMVFC_CAN_USE_NOOP_CMD 0x200
+#define IBMVFC_CAN_HANDLE_FPIN_EXT 0x800
__be64 node_name;
struct srp_direct_buf async;
u8 partition_name[IBMVFC_MAX_NAME];
@@ -233,6 +234,7 @@ struct ibmvfc_npiv_login_resp {
#define IBMVFC_SUPPORT_SCSI 0x0200
#define IBMVFC_SUPPORT_ASYNC_SUBQ 0x0800
#define IBMVFC_SUPPORT_NOOP_CMD 0x1000
+#define IBMVFC_SUPPORT_FPIN_EXT 0x2000
__be32 max_cmds;
__be32 scsi_id_sz;
__be64 max_dma_len;
@@ -715,6 +717,7 @@ struct ibmvfc_async_crq {
struct ibmvfc_async_subq {
volatile u8 valid;
#define IBMVFC_ASYNC_ID_IS_ASSOC_ID 0x01
+#define IBMVFC_ASYNC_IS_FPIN_EXT 0x02
#define IBMVFC_FC_EEH 0x04
#define IBMVFC_FC_FW_UPDATE 0x08
#define IBMVFC_FC_FW_DUMP 0x10
@@ -731,6 +734,34 @@ struct ibmvfc_async_subq {
} id;
} __packed __aligned(8);
+struct ibmvfc_fpin_data {
+#define IBMVFC_FPIN_EVENT_TYPE_VALID 0x01
+#define IBMVFC_FPIN_MODIFIER_VALID 0x02
+#define IBMVFC_FPIN_THRESHOLD_VALID 0x04
+#define IBMVFC_FPIN_SEVERITY_VALID 0x08
+#define IBMVFC_FPIN_EVENT_COUNT_VALID 0x10
+ u8 flags;
+ u8 reserved[3];
+ __be16 event_type;
+ __be16 event_type_modifier;
+ __be32 event_threshold;
+ union {
+ u8 severity;
+ __be32 event_count;
+ } event_data;
+} __packed __aligned(8);
+
+struct ibmvfc_async_subq_fpin {
+ volatile u8 valid;
+ u8 flags;
+ u8 link_state;
+ u8 fpin_status;
+ __be16 event;
+ __be16 pad;
+ volatile __be64 wwpn;
+ struct ibmvfc_fpin_data fpin_data;
+} __packed __aligned(8);
+
struct ibmvfc_async_work {
struct ibmvfc_host *vhost;
bool is_subq;
diff --git a/drivers/scsi/ibmvscsi/ibmvfc_kunit.c b/drivers/scsi/ibmvscsi/ibmvfc_kunit.c
index 86c1f6daca58..2f43916b43a5 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc_kunit.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc_kunit.c
@@ -3,6 +3,7 @@
#include <kunit/visibility.h>
#include <scsi/scsi_device.h>
#include <scsi/scsi_transport_fc.h>
+#include <scsi/fc/fc_els.h>
#include <linux/list.h>
#include <linux/delay.h>
#include "ibmvfc.h"
@@ -62,8 +63,6 @@ static void ibmvfc_async_fpin_test(struct kunit *test)
msleep(1U);
}
- msleep(500U);
-
post[IBMVFC_AE_FPIN_LINK_CONGESTED] = READ_ONCE(fc_host->fpin_stats.cn_device_specific);
post[IBMVFC_AE_FPIN_PORT_CONGESTED] = READ_ONCE(tgt->rport->fpin_stats.cn);
post[IBMVFC_AE_FPIN_PORT_CLEARED] = READ_ONCE(tgt->rport->fpin_stats.cn_clear);
@@ -116,8 +115,117 @@ static void ibmvfc_async_fpin_test(struct kunit *test)
post[IBMVFC_AE_FPIN_CONGESTION_CLEARED]);
}
+#define IBMVFC_TEST_FPIN_EXT(fs, ev, stat, crq) { \
+ crq.valid = 0x80; \
+ crq.flags = IBMVFC_ASYNC_IS_FPIN_EXT; \
+ crq.link_state = IBMVFC_AE_LS_LINK_UP; \
+ crq.fpin_status = (fs); \
+ crq.event = cpu_to_be16(IBMVFC_AE_FPIN); \
+ crq.wwpn = cpu_to_be64(tgt->wwpn); \
+ crq.fpin_data.flags = IBMVFC_FPIN_EVENT_TYPE_VALID; \
+ crq.fpin_data.event_type = cpu_to_be16((ev)); \
+ pre = READ_ONCE(tgt->rport->fpin_stats.stat); \
+ ibmvfc_handle_async((struct ibmvfc_crq *)&crq, vhost, true); \
+ msleep(1U); \
+ post = READ_ONCE(tgt->rport->fpin_stats.stat); \
+}
+
+/**
+ * ibmvfc_extended_fpin_test - unit test for extended FPIN events
+ * @test: pointer to kunit structure
+ *
+ * Tests
+ *
+ * Return: void
+ */
+static void ibmvfc_extended_fpin_test(struct kunit *test)
+{
+ enum ibmvfc_ae_fpin_status fs;
+ struct ibmvfc_async_subq_fpin crq[IBMVFC_AE_FPIN_CONGESTION_CLEARED+1] = {};
+ struct ibmvfc_async_subq_fpin
+ crqcn[IBMVFC_AE_FPIN_PORT_CONGESTED][FPIN_CONGN_DEVICE_SPEC+1] = {};
+ struct ibmvfc_async_subq_fpin crqportdg[FPIN_LI_DEVICE_SPEC+1] = {};
+ struct ibmvfc_target *tgt;
+ struct ibmvfc_host *vhost;
+ struct list_head *headp;
+ LIST_HEAD(evt_doneq);
+ u64 pre, post;
+
+ headp = ibmvfc_get_headp();
+ KUNIT_ASSERT_FALSE_MSG(test, list_empty(headp), "No ibmvfc devices available\n");
+ vhost = list_first_entry(headp, struct ibmvfc_host, queue);
+ KUNIT_ASSERT_GE_MSG(test, vhost->num_targets, 1, "No targets");
+
+ tgt = list_first_entry(&vhost->targets, struct ibmvfc_target, queue);
+ KUNIT_ASSERT_NOT_NULL(test, tgt->rport);
+
+ for (fs = IBMVFC_AE_FPIN_LINK_CONGESTED; fs <= IBMVFC_AE_FPIN_CONGESTION_CLEARED; fs++) {
+ switch (fs) {
+ case IBMVFC_AE_FPIN_PORT_CLEARED:
+ case IBMVFC_AE_FPIN_CONGESTION_CLEARED:
+ crq[fs].valid = 0x80;
+ crq[fs].flags = IBMVFC_ASYNC_IS_FPIN_EXT;
+ crq[fs].link_state = IBMVFC_AE_LS_LINK_UP;
+ crq[fs].fpin_status = fs;
+ crq[fs].event = cpu_to_be16(IBMVFC_AE_FPIN);
+ crq[fs].wwpn = cpu_to_be64(tgt->wwpn);
+ crq[fs].fpin_data.flags = IBMVFC_FPIN_EVENT_TYPE_VALID;
+ crq[fs].fpin_data.event_type = cpu_to_be16(FPIN_CONGN_CLEAR);
+ pre = READ_ONCE(tgt->rport->fpin_stats.cn_clear);
+ ibmvfc_handle_async((struct ibmvfc_crq *)&crq[fs], vhost, true);
+ msleep(1U);
+ post = READ_ONCE(tgt->rport->fpin_stats.cn_clear);
+ break;
+ case IBMVFC_AE_FPIN_LINK_CONGESTED:
+ case IBMVFC_AE_FPIN_PORT_CONGESTED:
+ IBMVFC_TEST_FPIN_EXT(fs, FPIN_CONGN_CLEAR, cn_clear,
+ crqcn[fs-1][FPIN_CONGN_CLEAR]);
+ IBMVFC_TEST_FPIN_EXT(fs, FPIN_CONGN_LOST_CREDIT,
+ cn_lost_credit,
+ crqcn[fs-1][FPIN_CONGN_LOST_CREDIT]);
+ IBMVFC_TEST_FPIN_EXT(fs, FPIN_CONGN_CREDIT_STALL,
+ cn_credit_stall,
+ crqcn[fs-1][FPIN_CONGN_CREDIT_STALL]);
+ IBMVFC_TEST_FPIN_EXT(fs, FPIN_CONGN_OVERSUBSCRIPTION,
+ cn_oversubscription,
+ crqcn[fs-1][FPIN_CONGN_OVERSUBSCRIPTION]);
+ IBMVFC_TEST_FPIN_EXT(fs, FPIN_CONGN_DEVICE_SPEC,
+ cn_device_specific,
+ crqcn[fs-1][FPIN_CONGN_DEVICE_SPEC]);
+ break;
+ case IBMVFC_AE_FPIN_PORT_DEGRADED:
+ IBMVFC_TEST_FPIN_EXT(fs, FPIN_LI_UNKNOWN,
+ li_failure_unknown,
+ crqportdg[FPIN_LI_UNKNOWN]);
+ IBMVFC_TEST_FPIN_EXT(fs, FPIN_LI_LINK_FAILURE,
+ li_link_failure_count,
+ crqportdg[FPIN_LI_LINK_FAILURE]);
+ IBMVFC_TEST_FPIN_EXT(fs, FPIN_LI_LOSS_OF_SYNC,
+ li_loss_of_sync_count,
+ crqportdg[FPIN_LI_LOSS_OF_SYNC]);
+ IBMVFC_TEST_FPIN_EXT(fs, FPIN_LI_LOSS_OF_SIG,
+ li_loss_of_signals_count,
+ crqportdg[FPIN_LI_LOSS_OF_SIG]);
+ IBMVFC_TEST_FPIN_EXT(fs, FPIN_LI_PRIM_SEQ_ERR,
+ li_prim_seq_err_count,
+ crqportdg[FPIN_LI_PRIM_SEQ_ERR]);
+ IBMVFC_TEST_FPIN_EXT(fs, FPIN_LI_INVALID_TX_WD,
+ li_invalid_tx_word_count,
+ crqportdg[FPIN_LI_INVALID_TX_WD]);
+ IBMVFC_TEST_FPIN_EXT(fs, FPIN_LI_INVALID_CRC,
+ li_invalid_crc_count,
+ crqportdg[FPIN_LI_INVALID_CRC]);
+ IBMVFC_TEST_FPIN_EXT(fs, FPIN_LI_DEVICE_SPEC,
+ li_device_specific,
+ crqportdg[FPIN_LI_DEVICE_SPEC]);
+ break;
+ }
+ }
+}
+
static struct kunit_case ibmvfc_fpin_test_cases[] = {
- KUNIT_CASE_SLOW(ibmvfc_async_fpin_test),
+ KUNIT_CASE(ibmvfc_async_fpin_test),
+ KUNIT_CASE(ibmvfc_extended_fpin_test),
{},
};
--
2.55.0
^ permalink raw reply related [flat|nested] 17+ messages in thread
* Re: [PATCH v4 3/8] ibmvfc: make ibmvfc login to fabric
2026-07-10 19:16 ` [PATCH v4 3/8] ibmvfc: make ibmvfc login to fabric Dave Marquardt via B4 Relay
@ 2026-07-10 19:30 ` sashiko-bot
0 siblings, 0 replies; 17+ messages in thread
From: sashiko-bot @ 2026-07-10 19:30 UTC (permalink / raw)
To: Dave Marquardt; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The newly introduced IBMVFC_YES_SCSI capability flag is defined but never added to the client's login capabilities payload, preventing the client from advertising SCSI fabric login support to the VIOS.
--
--- Patch [3]: [PATCH v4 3/8] ibmvfc: make ibmvfc login to fabric ---
commit 1b5a2938bf5a0e21e24c77be11449d47426c29c4
Author: Dave Marquardt <davemarq@linux.ibm.com>
ibmvfc: make ibmvfc login to fabric
Add fabric login capability to support asynchronous event queue with
dedicated interrupt as required by NPIV specification for async sub-queue
and full/extended FPIN message support.
[ ... ]
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.h b/drivers/scsi/ibmvscsi/ibmvfc.h
> index 526632cb72377..adfd67e85af86 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc.h
> +++ b/drivers/scsi/ibmvscsi/ibmvfc.h
> @@ -180,6 +181,7 @@ struct ibmvfc_npiv_login {
> #define IBMVFC_CAN_HANDLE_FPIN 0x004
> #define IBMVFC_CAN_USE_MAD_VERSION 0x008
> #define IBMVFC_CAN_SEND_VF_WWPN 0x010
> +#define IBMVFC_YES_SCSI 0x040
[Severity: Medium]
Will the new fabric login functionality be reachable?
The IBMVFC_YES_SCSI flag is defined here, but it doesn't appear to be added
to the login capabilities payload in ibmvfc_set_login_info():
login_info->capabilities =
cpu_to_be64(IBMVFC_CAN_MIGRATE | IBMVFC_CAN_SEND_VF_WWPN |
IBMVFC_CAN_USE_NOOP_CMD);
Without advertising this capability to the VIOS, will the VIOS ever return
the IBMVFC_SUPPORT_SCSI flag, or will the host always fall back to the
legacy initialization path?
> #define IBMVFC_CAN_USE_NOOP_CMD 0x200
> __be64 node_name;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-ibmvfc-fpin-support-v4-0-ef031ac19520@linux.ibm.com?part=3
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 1/8] ibmvfc: add basic FPIN support
2026-07-10 19:16 ` [PATCH v4 1/8] ibmvfc: add basic FPIN support Dave Marquardt via B4 Relay
@ 2026-07-10 19:31 ` sashiko-bot
0 siblings, 0 replies; 17+ messages in thread
From: sashiko-bot @ 2026-07-10 19:31 UTC (permalink / raw)
To: Dave Marquardt; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [Critical] Removal of `async->valid = 0;` leads to an infinite loop and stale message processing in the async CRQ tasklet.
- [High] Missing reference count acquisition on `tgt` in `ibmvfc_process_async_work` leads to Use-After-Free.
- [High] Improper devres lifecycle management of `fpin_workq` causes a double-free on probe failure and a Use-After-Free during device removal.
- [Medium] Data races and potential use-after-free in the new KUnit test module due to lockless access of global and host device lists.
--
--- Patch [1]: [PATCH v4 1/8] ibmvfc: add basic FPIN support ---
commit d8410cbccc8b5f74d804dc594b4dd758bd093c6e
Author: Dave Marquardt <davemarq@linux.ibm.com>
ibmvfc: add basic FPIN support
This patch implements basic Fabric Performance Impact Notifications (FPIN)
support to enable monitoring of fabric congestion and link integrity events.
It adds async event handling that offloads FPIN processing to a workqueue
and introduces a KUnit test module for validation.
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
[ ... ]
> @@ -3185,16 +3189,228 @@ static const char *ibmvfc_get_link_state(enum ibmvfc_ae_link_state state)
> +static void ibmvfc_process_async_work(struct work_struct *work)
> +{
[ ... ]
> + spin_lock_irqsave(vhost->host->host_lock, flags);
> + list_for_each_entry_safe(tgt, next, &vhost->targets, queue) {
> + if (crq->scsi_id && cpu_to_be64(tgt->scsi_id) != crq->scsi_id)
> + continue;
> + if (crq->wwpn && cpu_to_be64(tgt->ids.port_name) != crq->wwpn)
> + continue;
> + if (crq->node_name && cpu_to_be64(tgt->ids.node_name) != crq->node_name)
> + continue;
> + if (!tgt->rport)
> + continue;
> + break;
> + }
> + spin_unlock_irqrestore(vhost->host->host_lock, flags);
> +
> + if (list_entry_is_head(tgt, &vhost->targets, queue) || !tgt->rport) {
> + dev_err_ratelimited(vhost->dev, "Invalid target for FPIN\n");
> + goto end;
> + }
> +
> + fpin = ibmvfc_basic_fpin_to_desc(crq, tgt->wwpn);
[Severity: High]
Does this leave the target pointer vulnerable to a use-after-free?
The lock protecting the vhost->targets list is dropped before we access
tgt->wwpn and tgt->rport, without acquiring a reference via kref_get().
If another thread handles a target logout or hotplug concurrently, it could
remove and free the target structure, causing memory corruption when we
attempt to access it here.
[ ... ]
> @@ -3803,8 +4031,6 @@ static void ibmvfc_tasklet(void *data)
> /* Pull all the valid messages off the async CRQ */
> while ((async = ibmvfc_next_async_crq(vhost)) != NULL) {
> ibmvfc_handle_async(async, vhost);
> - async->valid = 0;
> - wmb();
> }
[Severity: Critical]
Could removing this clearing operation lead to an infinite loop?
The hardware signals a valid entry by setting the valid field to 0x80. By
removing the clearing of async->valid, the driver's ring-buffer polling logic
will mistakenly see the old, uncleared entries as new valid entries once the
circular queue index wraps around.
This would trap the system in an infinite loop within softirq context when
processing any valid async event.
[ ... ]
> @@ -6364,9 +6588,15 @@ static int ibmvfc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
[ ... ]
> - if ((rc = ibmvfc_alloc_mem(vhost)))
> + vhost->fpin_workq = devm_alloc_workqueue(vhost->dev, "%s-fpin-workq-%u", 0, 0,
> + IBMVFC_NAME, shost->host_no);
> + if (vhost->fpin_workq == NULL)
> goto free_scsi_host;
[ ... ]
> @@ -6412,6 +6642,9 @@ static int ibmvfc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
> kthread_stop(vhost->work_thread);
> free_host_mem:
> ibmvfc_free_mem(vhost);
> +free_workq:
> + destroy_workqueue(vhost->fpin_workq);
> + vhost->fpin_workq = NULL;
[Severity: High]
Can this result in a double-free on probe failure, and a use-after-free
during device removal?
The workqueue is allocated using devm_alloc_workqueue(). On probe failure,
destroy_workqueue() is called explicitly, and then devres will automatically
call it again during cleanup, causing a double-free.
Additionally, during device removal in ibmvfc_remove(), the host is manually
freed before devres automatically cleans up the workqueue. If there are
pending work items like ibmvfc_process_async_work(), they will execute and
attempt to dereference host pointers that have already been freed.
[ ... ]
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc_kunit.c b/drivers/scsi/ibmvscsi/ibmvfc_kunit.c
> --- /dev/null
> +++ b/drivers/scsi/ibmvscsi/ibmvfc_kunit.c
[ ... ]
> +static void ibmvfc_async_fpin_test(struct kunit *test)
> +{
[ ... ]
> + headp = ibmvfc_get_headp();
> + if (list_empty(headp))
> + kunit_skip(test, "No ibmvfc devices available");
> + queue = headp->next;
> + vhost = container_of(queue, struct ibmvfc_host, queue);
> +
> + KUNIT_ASSERT_GE_MSG(test, vhost->num_targets, 1, "No targets");
> + tgt = list_first_entry(&vhost->targets, struct ibmvfc_target, queue);
[Severity: Medium]
Is it safe to access the global ibmvfc_head list and vhost->targets list
without holding the appropriate locks?
The test traverses the global ibmvfc_head without acquiring the
ibmvfc_driver_lock, and then iterates over vhost->targets without acquiring
the vhost->host->host_lock.
Since KUnit tests can run dynamically while other processes or the hypervisor
interact with the driver, concurrently adding or removing devices could modify
these lists and free structures, resulting in a use-after-free.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-ibmvfc-fpin-support-v4-0-ef031ac19520@linux.ibm.com?part=1
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 2/8] ibmvfc: Add NOOP command support
2026-07-10 19:16 ` [PATCH v4 2/8] ibmvfc: Add NOOP command support Dave Marquardt via B4 Relay
@ 2026-07-10 19:31 ` sashiko-bot
0 siblings, 0 replies; 17+ messages in thread
From: sashiko-bot @ 2026-07-10 19:31 UTC (permalink / raw)
To: Dave Marquardt; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [Medium] The logic for logging unexpected NOOP messages contradicts the commit message due to a state check that suppresses early NOOP logs.
Pre-existing issues:
- [Critical] In `ibmvfc_handle_scrq`, an unvalidated or forged `crq->ioba` token from the VIOS causes a kernel panic or arbitrary code execution because it is cast to an `evt` pointer and dereferenced before validation.
--
--- Patch [2]: [PATCH v4 2/8] ibmvfc: Add NOOP command support ---
commit 06afd3b464e63134568df7f26e9d5bc73cb9e427
Author: Dave Marquardt <davemarq@linux.ibm.com>
Subject: ibmvfc: Add NOOP command support
This commit adds support for VFC_NOOP messages from VIOS to enable keep-alive
functionality between the client and server. It sets the
IBMVFC_CAN_USE_NOOP_CMD capability bit during NPIV login and defines the
VFC_NOOP CRQ format handling.
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
> index d3fd1d3437c69..a7e3b7ee06835 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
> @@ -3569,6 +3571,14 @@ static void ibmvfc_handle_crq(struct ibmvfc_crq *crq, struct ibmvfc_host *vhost,
> if (crq->format == IBMVFC_ASYNC_EVENT)
> return;
>
> + if (crq->format == IBMVFC_VFC_NOOP) {
> + if (vhost->state == IBMVFC_ACTIVE &&
> + !ibmvfc_check_caps(vhost, IBMVFC_SUPPORT_NOOP_CMD))
> + dev_err_ratelimited(vhost->dev,
> + "Received unexpected NOOP command from partner\n");
[Severity: Medium]
Does this logic contradict the commit message? The commit message states we
should log unexpected NOOP messages if received before VIOS advertises
support during NPIV login.
However, checking if the host state is IBMVFC_ACTIVE appears to actively
suppress the error log if the host is still initializing during NPIV login.
Any NOOP messages received in the pre-login window would silently fail this
check and be dropped without logging.
> + return;
> + }
> +
> [ ... ]
> @@ -4095,7 +4105,15 @@ static void ibmvfc_handle_scrq(struct ibmvfc_crq *crq, struct ibmvfc_host *vhost
> case IBMVFC_CRQ_XPORT_EVENT:
> return;
> default:
> - dev_err(vhost->dev, "Got and invalid message type 0x%02x\n", crq->valid);
> + dev_err(vhost->dev, "Got an invalid message type 0x%02x\n", crq->valid);
> + return;
> + }
> +
> + if (crq->format == IBMVFC_VFC_NOOP)
> + return;
> +
> + if (unlikely(!evt)) {
> + dev_err(vhost->dev, "Received null event\n");
> return;
> }
[Severity: Critical]
This is a pre-existing issue, but can an unvalidated or forged token from
the VIOS cause a kernel panic or arbitrary code execution here?
In ibmvfc_handle_scrq(), the token is cast directly to an evt pointer.
Before evt is validated using ibmvfc_valid_event(), it appears the code
accesses evt->queue->evt_pool.
If the token is maliciously crafted, could dereferencing evt->queue cause a
page fault? Furthermore, could a fake queue structure in guest memory bypass
the bounds check, allowing a forged event to be placed on the queue and
executing arbitrary code when evt->done(evt) is called later in
ibmvfc_drain_sub_crq()?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-ibmvfc-fpin-support-v4-0-ef031ac19520@linux.ibm.com?part=2
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 7/8] ibmvfc: register and use asynchronous sub-queue for events
2026-07-10 19:16 ` [PATCH v4 7/8] ibmvfc: register and use asynchronous sub-queue for events Dave Marquardt via B4 Relay
@ 2026-07-10 19:32 ` sashiko-bot
0 siblings, 0 replies; 17+ messages in thread
From: sashiko-bot @ 2026-07-10 19:32 UTC (permalink / raw)
To: Dave Marquardt; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Failure to clear `scrq->irq` on error and unconditional teardown of sub-CRQs causes double-free of IRQs and hypervisor resources.
--
--- Patch [7]: [PATCH v4 7/8] ibmvfc: register and use asynchronous sub-queue for events ---
commit 381f7f27a85750da81e10927e42035d25ca1d22c
Author: Dave Marquardt <davemarq@linux.ibm.com>
ibmvfc: register and use asynchronous sub-queue for events
Complete async sub-queue integration by setting up interrupt handling,
registering the queue as a channel, and enabling its use during NPIV login.
Register async_scrq during channel initialization and unregister during
cleanup.
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
> index ee56f13f1a979..eb786ac242745 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
[ ... ]
> @@ -6470,7 +6571,8 @@ static void ibmvfc_dereg_sub_crqs(struct ibmvfc_host *vhost,
> return;
>
> for (i = 0; i < channels->max_queues; i++)
> - ibmvfc_deregister_channel(vhost, channels, i);
> + ibmvfc_deregister_channel(vhost, channels, &channels->scrqs[i]);
> + ibmvfc_deregister_channel(vhost, channels, channels->async_scrq);
[Severity: High]
This isn't a bug introduced by this patch, but does adding the async_scrq to
this unconditional deregistration loop extend a double-free bug?
If request_irq() fails during sub-CRQ registration in
ibmvfc_register_channel_common(), it calls irq_dispose_mapping(scrq->irq)
and goes to irq_failed to free the sub-CRQ. However, it doesn't clear
scrq->irq or scrq->cookie.
Later, ibmvfc_dereg_sub_crqs() iterates unconditionally over all queues
(including the failed one) and calls ibmvfc_deregister_channel(), which
again calls free_irq(), irq_dispose_mapping(), and
plpar_hcall_norets(H_FREE_SUB_CRQ).
Could this lead to IRQ domain corruption and a hypervisor resource
double-free?
>
> LEAVE;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-ibmvfc-fpin-support-v4-0-ef031ac19520@linux.ibm.com?part=7
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 5/8] ibmvfc: allocate asynchronous sub-queue
2026-07-10 19:16 ` [PATCH v4 5/8] ibmvfc: allocate " Dave Marquardt via B4 Relay
@ 2026-07-10 19:36 ` sashiko-bot
0 siblings, 0 replies; 17+ messages in thread
From: sashiko-bot @ 2026-07-10 19:36 UTC (permalink / raw)
To: Dave Marquardt; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] The newly allocated asynchronous sub-queue (`async_scrq`) is never registered with the hypervisor, leaving its cookie uninitialized and the queue non-functional.
Pre-existing issues:
- [High] Unprotected dereference of queue pointers in `ibmvfc_channel_setup()` leads to a kernel panic if MQ is disabled or if queue allocation fails.
--
--- Patch [5]: [PATCH v4 5/8] ibmvfc: allocate asynchronous sub-queue ---
commit d243c6fd5b265f83cb24c3737fb1dfe6fd989a93
Author: Dave Marquardt <davemarq@linux.ibm.com>
ibmvfc: allocate asynchronous sub-queue
This commit allocates and initializes an asynchronous sub-queue required for
receiving full and extended FPIN events from the VIOS.
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
[ ... ]
> @@ -5425,6 +5428,7 @@ static void ibmvfc_channel_setup(struct ibmvfc_host *vhost)
> setup_buf->num_scsi_subq_channels = cpu_to_be32(num_channels);
> for (i = 0; i < num_channels; i++)
> setup_buf->channel_handles[i] = cpu_to_be64(scrqs->scrqs[i].cookie);
> + setup_buf->async_subq_handle = cpu_to_be64(scrqs->async_scrq->cookie);
[Severity: High]
This is a pre-existing issue, but does this code unconditionally dereference
the queue pointers?
If MQ is disabled or if queue allocation fails in ibmvfc_alloc_channels(),
scrqs->scrqs and scrqs->async_scrq are left as NULL.
If the VIOS subsequently indicates channel support in its capabilities,
ibmvfc_channel_setup() is called and these unallocated pointers are
dereferenced here, which would result in a kernel panic.
Could we add a check to ensure the queues were successfully allocated before
dereferencing them during channel setup?
[ ... ]
> @@ -6392,6 +6396,17 @@ static int ibmvfc_alloc_channels(struct ibmvfc_host *vhost,
> if (!channels->scrqs)
> return -ENOMEM;
>
> + channels->async_scrq = kzalloc_obj(*channels->async_scrq, GFP_KERNEL);
> + if (!channels->async_scrq) {
> + rc = -ENOMEM;
> + goto free_scrqs;
> + }
> +
> + rc = ibmvfc_alloc_queue(vhost, channels->async_scrq,
> + IBMVFC_SUB_CRQ_FMT);
> + if (rc)
> + goto free_async;
[Severity: High]
Is this missing a step to register the asynchronous sub-queue with the
hypervisor?
The queue is allocated here and mapped in memory, but it doesn't appear to
be registered via h_reg_sub_crq() anywhere.
Because it is never registered, its cookie remains 0, meaning an
uninitialized handle is sent to the VIOS during ibmvfc_channel_setup(),
and no hardware IRQ is mapped or requested, which renders the async queue
non-functional.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-ibmvfc-fpin-support-v4-0-ef031ac19520@linux.ibm.com?part=5
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 6/8] ibmvfc: extend async event handlers to handle async sub queue events
2026-07-10 19:16 ` [PATCH v4 6/8] ibmvfc: extend async event handlers to handle async sub queue events Dave Marquardt via B4 Relay
@ 2026-07-10 19:37 ` sashiko-bot
0 siblings, 0 replies; 17+ messages in thread
From: sashiko-bot @ 2026-07-10 19:37 UTC (permalink / raw)
To: Dave Marquardt; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] The driver ignores the `IBMVFC_ASYNC_ID_IS_ASSOC_ID` flag when processing sub-queue events, incorrectly interpreting association IDs as node names.
- [High] Hardcoded event type in `ibmvfc_full_fpin_to_desc()` drops FPIN congestion event classification.
Pre-existing issues:
- [High] Double-free and use-after-free of the FPIN workqueue due to incorrect mixing of `devm_alloc_workqueue()` and manual lifecycle management.
--
--- Patch [6]: [PATCH v4 6/8] ibmvfc: extend async event handlers to handle async sub queue events ---
commit f425472451b9ed0df3bb74b15fd60bb31167f05e
Author: Dave Marquardt <davemarq@linux.ibm.com>
ibmvfc: extend async event handlers to handle async sub queue events
Refactor async event handling to support both traditional async CRQs and
new asynchronous sub-queue CRQs.
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
> index 586847ff3336d..ee56f13f1a979 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
[ ... ]
> @@ -3349,6 +3349,27 @@ ibmvfc_basic_fpin_to_desc(struct ibmvfc_async_crq *crq, u64 wwpn)
> cpu_to_be32(1));
> }
>
> +/**
> + * ibmvfc_full_fpin_to_desc(): allocate and populate a struct fc_els_fpin struct
> + * containing a descriptor.
> + * @ibmvfc_fpin: Pointer to async subq FPIN data
> + *
> + * Allocate a struct fc_els_fpin containing a descriptor and populate
> + * based on data from *ibmvfc_fpin.
> + *
> + * Return:
> + * NULL - unable to allocate structure
> + * non-NULL - pointer to populated struct fc_els_fpin
> + */
> +static struct fc_els_fpin *
> +ibmvfc_full_fpin_to_desc(struct ibmvfc_async_subq *ibmvfc_fpin)
> +{
> + return ibmvfc_common_fpin_to_desc(ibmvfc_fpin->fpin_status, ibmvfc_fpin->wwpn,
> + cpu_to_be16(0), cpu_to_be16(0),
> + cpu_to_be32(IBMVFC_FPIN_DEFAULT_EVENT_THRESHOLD),
> + cpu_to_be32(1));
> +}
[Severity: High]
Is it intentional to hardcode the event type to cpu_to_be16(0) here?
Unlike ibmvfc_basic_fpin_to_desc() which maps the fpin_status to specific
types like FPIN_CONGN_DEVICE_SPEC, this appears to drop the FPIN congestion
event classification.
Could this prevent the FC transport layer from updating the storage
congestion statistics correctly?
[ ... ]
> @@ -3356,73 +3377,131 @@ static void ibmvfc_process_async_work(struct work_struct *work)
[ ... ]
> + if (crq) {
> + wwpn = crq->wwpn;
> + node_name = crq->node_name;
> + scsi_id = crq->scsi_id;
> + } else {
> + wwpn = subq->wwpn;
> + node_name = subq->id.node_name;
> + scsi_id = 0;
> + }
[Severity: High]
Does this code need to check the IBMVFC_ASYNC_ID_IS_ASSOC_ID flag before
reading subq->id.node_name?
If the VIOS provides an association ID instead of a node name and sets the
flag in subq->flags, it seems the union would contain an assoc_id.
Could reading this unconditionally as a node_name cause target lookups to fail
and silently drop events?
>
> - if (!crq->scsi_id && !crq->wwpn && !crq->node_name)
> - goto end;
> + if (!scsi_id && !wwpn && !node_name)
> + goto free;
>
> spin_lock_irqsave(vhost->host->host_lock, flags);
[Severity: High]
This is a pre-existing issue, but is it safe to use this lock here during
driver removal?
If the vhost->fpin_workq was allocated with devm_alloc_workqueue(), it won't
be destroyed until devres runs after ibmvfc_remove() completes.
Since ibmvfc_remove() calls scsi_host_put(vhost->host), could an async FPIN
event executing concurrently access the freed host lock? Also, on probe
error paths, if destroy_workqueue() is called manually on a devm-managed
workqueue, could this lead to a double-free when devres cleans it up?
[ ... ]
> @@ -3460,18 +3539,18 @@ VISIBLE_IF_KUNIT void ibmvfc_handle_async(void *crq,
[ ... ]
> + if (is_subq) {
> + subq = crq;
> + event = be16_to_cpu(subq->event);
> + link_state = subq->link_state;
> + scsi_id = 0;
> + wwpn = subq->wwpn;
> + node_name = subq->id.node_name;
[Severity: High]
Similarly, does ibmvfc_handle_async() need to verify the
IBMVFC_ASYNC_ID_IS_ASSOC_ID flag before extracting the node_name?
If the subq->id union contains an association ID instead of a WWNN, could
this result in failed target lookups for ELS_LOGO, PRLO, and PLOGI events?
> + } else {
> + async_crq = crq;
> + event = be64_to_cpu(async_crq->event);
> + link_state = async_crq->link_state;
> + scsi_id = async_crq->scsi_id;
> + wwpn = async_crq->wwpn;
> + node_name = async_crq->node_name;
> + }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-ibmvfc-fpin-support-v4-0-ef031ac19520@linux.ibm.com?part=6
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 4/8] ibmvfc: define asynchronous sub-queue
2026-07-10 19:16 ` [PATCH v4 4/8] ibmvfc: define asynchronous sub-queue Dave Marquardt via B4 Relay
@ 2026-07-10 19:40 ` sashiko-bot
0 siblings, 0 replies; 17+ messages in thread
From: sashiko-bot @ 2026-07-10 19:40 UTC (permalink / raw)
To: Dave Marquardt; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
New issues:
- [High] The driver unloads without explicitly flushing or canceling `fpin_workq`, leading to a Use-After-Free.
- [High] The error path in `ibmvfc_probe()` manually destroys a device-managed workqueue, resulting in a double-free.
- [High] The `ibmvfc_process_async_work()` function accesses a target object outside of the host lock without holding a reference count, leading to a TOCTOU Use-After-Free.
- [High] The driver fails to clear the `valid` byte of asynchronous CRQ entries after processing them, which will permanently stall asynchronous events.
Pre-existing issues:
- [High] Out-of-bounds array write when parsing channel setup completion from VIOS.
--
--- Patch [4]: [PATCH v4 4/8] ibmvfc: define asynchronous sub-queue ---
commit 9bd2cdf24ef9c308bf4803492cdf8f30fbef4057
Author: Dave Marquardt <davemarq@linux.ibm.com>
ibmvfc: define asynchronous sub-queue
Define data structures for asynchronous sub-queue support required for full and
extended FPIN functionality. Introduce the ibmvfc_async_subq structure for
async events, update channel setup capabilities, and negotiate async support
with VIOS during login.
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
> index 3dd2adda195ee..c95e78d729ed7 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
[ ... ]
> @@ -3185,16 +3191,228 @@ static const char *ibmvfc_get_link_state(enum ibmvfc_ae_link_state state)
[ ... ]
> +static void ibmvfc_process_async_work(struct work_struct *work)
> +{
> + struct ibmvfc_target *tgt, *next;
> + struct ibmvfc_async_work *aw;
> + struct ibmvfc_async_crq *crq;
> + struct ibmvfc_host *vhost;
> + struct fc_els_fpin *fpin;
> + unsigned long flags;
> +
> + aw = container_of(work, struct ibmvfc_async_work, async_work_s);
> + crq = &aw->crq;
> + vhost = aw->vhost;
> +
> + if (!crq->scsi_id && !crq->wwpn && !crq->node_name)
> + goto end;
> +
> + spin_lock_irqsave(vhost->host->host_lock, flags);
> + list_for_each_entry_safe(tgt, next, &vhost->targets, queue) {
> + if (crq->scsi_id && cpu_to_be64(tgt->scsi_id) != crq->scsi_id)
> + continue;
> + if (crq->wwpn && cpu_to_be64(tgt->ids.port_name) != crq->wwpn)
> + continue;
> + if (crq->node_name && cpu_to_be64(tgt->ids.node_name) != crq->node_name)
> + continue;
> + if (!tgt->rport)
> + continue;
> + break;
> + }
> + spin_unlock_irqrestore(vhost->host->host_lock, flags);
> +
> + if (list_entry_is_head(tgt, &vhost->targets, queue) || !tgt->rport) {
> + dev_err_ratelimited(vhost->dev, "Invalid target for FPIN\n");
> + goto end;
> + }
> +
> + fpin = ibmvfc_basic_fpin_to_desc(crq, tgt->wwpn);
[Severity: High]
What prevents a use-after-free here if a concurrent device deletion or logout
frees tgt immediately after the lock is dropped?
Since the lock is dropped without acquiring a reference to tgt (via kref_get),
is it safe to access tgt->wwpn or pass tgt->vhost->host to fc_host_fpin_rcv()
outside of the lock?
[ ... ]
> @@ -3803,8 +4041,6 @@ static void ibmvfc_tasklet(void *data)
> /* Pull all the valid messages off the async CRQ */
> while ((async = ibmvfc_next_async_crq(vhost)) != NULL) {
> ibmvfc_handle_async(async, vhost);
> - async->valid = 0;
> - wmb();
> }
[Severity: High]
Is it intentional to remove the clearing of the valid byte here?
The VIO CRQ protocol normally requires clearing the valid byte after consuming
an entry. Without this, it seems the consumed entries are never freed, which
could cause the async CRQ ring buffer to permanently fill up and stall
asynchronous events.
>
> /* Pull all the valid messages off the CRQ */
> @@ -3818,8 +4054,6 @@ static void ibmvfc_tasklet(void *data)
> if ((async = ibmvfc_next_async_crq(vhost)) != NULL) {
> vio_disable_interrupts(vdev);
> ibmvfc_handle_async(async, vhost);
> - async->valid = 0;
> - wmb();
> } else if ((crq = ibmvfc_next_crq(vhost)) != NULL) {
[ ... ]
> @@ -5069,8 +5393,12 @@ static void ibmvfc_channel_setup_done(struct ibmvfc_event *evt)
> return;
> }
>
> - ibmvfc_set_host_action(vhost, IBMVFC_HOST_ACTION_QUERY);
> - wake_up(&vhost->work_wait_q);
> + if (ibmvfc_check_caps(vhost, IBMVFC_SUPPORT_SCSI)) {
> + ibmvfc_fabric_login(vhost);
> + } else {
> + ibmvfc_set_host_action(vhost, IBMVFC_HOST_ACTION_QUERY);
> + wake_up(&vhost->work_wait_q);
> + }
> }
[Severity: High]
This isn't a bug introduced by this patch, but is there a bounds check
missing for active_queues in this function?
Looking earlier in ibmvfc_channel_setup_done(), active_queues is read from
the VIOS hardware response and used as a loop bound:
ibmvfc_channel_setup_done() {
...
active_queues = be32_to_cpu(setup->num_scsi_subq_channels);
scrqs->active_queues = active_queues;
...
for (i = 0; i < active_queues; i++)
scrqs->scrqs[i].vios_cookie = be64_to_cpu(setup->channel_handles[i]);
...
}
Could this loop write past the end of the scrqs->scrqs array if the VIOS
returns a value greater than scrqs->max_queues?
[ ... ]
> @@ -6364,9 +6694,15 @@ static int ibmvfc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
> INIT_WORK(&vhost->rport_add_work_q, ibmvfc_rport_add_thread);
> mutex_init(&vhost->passthru_mutex);
>
> - if ((rc = ibmvfc_alloc_mem(vhost)))
> + vhost->fpin_workq = devm_alloc_workqueue(vhost->dev, "%s-fpin-workq-%u", 0, 0,
> + IBMVFC_NAME, shost->host_no);
> + if (vhost->fpin_workq == NULL)
> goto free_scsi_host;
>
[Severity: High]
Could this devm_alloc_workqueue() allocation lead to a use-after-free during
driver unload?
During driver unload, ibmvfc_remove() calls scsi_host_put() which frees the
Scsi_Host and vhost structures. Since devres callbacks run after the remove
function returns, the workqueue will be destroyed after vhost is already
freed. If any pending ibmvfc_process_async_work() items execute, they would
dereference the freed vhost structure.
> + rc = ibmvfc_alloc_mem(vhost);
> + if (rc)
> + goto free_workq;
> +
> vhost->work_thread = kthread_run(ibmvfc_work, vhost, "%s_%d", IBMVFC_NAME,
> shost->host_no);
>
> @@ -6412,6 +6748,9 @@ static int ibmvfc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
> kthread_stop(vhost->work_thread);
> free_host_mem:
> ibmvfc_free_mem(vhost);
> +free_workq:
> + destroy_workqueue(vhost->fpin_workq);
> + vhost->fpin_workq = NULL;
[Severity: High]
Will manually destroying a devm-managed workqueue here cause a double-free?
Because fpin_workq was allocated with devm_alloc_workqueue(), devres
automatically registers an action to destroy it. If this error path manually
calls destroy_workqueue(), it appears devres will try to destroy it a second
time during automatic cleanup.
> free_scsi_host:
> scsi_host_put(shost);
> out:
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-ibmvfc-fpin-support-v4-0-ef031ac19520@linux.ibm.com?part=4
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCH v4 8/8] ibmvfc: handle extended FPIN events
2026-07-10 19:16 ` [PATCH v4 8/8] ibmvfc: handle extended FPIN events Dave Marquardt via B4 Relay
@ 2026-07-10 19:40 ` sashiko-bot
0 siblings, 0 replies; 17+ messages in thread
From: sashiko-bot @ 2026-07-10 19:40 UTC (permalink / raw)
To: Dave Marquardt; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] Extended FPIN events are incorrectly dropped because `node_name` is populated with garbage data from overlapping `fpin_data` union fields, causing target lookup to fail.
- [High] The async workqueue `fpin_workq` is not properly flushed or destroyed before the `vhost` memory is freed during driver removal, leading to a Use-After-Free.
- [Medium] The newly added KUnit tests for extended FPIN events omit assertions and use zero-initialized data that masks the `node_name` extraction bug.
- [Medium] KUnit tests use an arbitrary 1ms sleep to wait for asynchronous work queues to complete, causing test flakiness.
--
--- Patch [8]: [PATCH v4 8/8] ibmvfc: handle extended FPIN events ---
commit a18cc405b1579697dec2f0dfe75048a6e10f4167
Author: Dave Marquardt <davemarq@linux.ibm.com>
ibmvfc: handle extended FPIN events
Implement support for extended FPIN messages via the asynchronous sub-queue.
This adds parsing for detailed fabric events, updates capability
advertisement, and includes KUnit validation for the new functionality.
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.c b/drivers/scsi/ibmvscsi/ibmvfc.c
> index eb786ac242745..b131a4a3041b7 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc.c
[ ... ]
> @@ -3372,12 +3373,48 @@ ibmvfc_full_fpin_to_desc(struct ibmvfc_async_subq *ibmvfc_fpin)
> cpu_to_be32(1));
> }
>
> +/**
> + * ibmvfc_ext_fpin_to_desc(): allocate and populate a struct fc_els_fpin struct
[ ... ]
> /**
> * ibmvfc_process_async_work - Process IBMVFC_AE_FPIN async CRQ from work queue
> * @work: pointer to work_struct
> */
> static void ibmvfc_process_async_work(struct work_struct *work)
> {
> + struct ibmvfc_async_subq_fpin *sqfpin;
> struct ibmvfc_target *tgt, *next;
[Severity: High]
Does this leave the work queue active during driver removal?
In ibmvfc_remove(), the driver drops the last reference to the SCSI host via
scsi_host_put(vhost->host), which frees the shost and the embedded vhost
structure. However, fpin_workq is allocated via devm_alloc_workqueue(). The
devres cleanup runs after ibmvfc_remove() returns.
If destroy_workqueue() drains pending work during devm teardown, it could
execute ibmvfc_process_async_work(), which accesses the freed vhost memory
when taking the host lock: spin_lock_irqsave(vhost->host->host_lock, flags).
Could we flush or destroy the work queue explicitly in ibmvfc_remove() before
releasing the SCSI host?
[ ... ]
> @@ -3439,8 +3476,20 @@ static void ibmvfc_process_async_work(struct work_struct *work)
>
> if (crq)
> fpin = ibmvfc_basic_fpin_to_desc(crq, tgt->wwpn);
> - else
> - fpin = ibmvfc_full_fpin_to_desc(subq);
> + else {
> + sqfpin = (struct ibmvfc_async_subq_fpin *)subq;
[Severity: High]
Does this drop extended FPIN events by matching on garbage data?
For an extended FPIN event, struct ibmvfc_async_subq and
struct ibmvfc_async_subq_fpin share memory layout. Earlier in
ibmvfc_process_async_work(), the code extracts:
wwpn = subq->wwpn;
node_name = subq->id.node_name;
The subq->id.node_name overlaps with fpin_data.event_threshold and
fpin_data.event_data.event_count. Since these data values are typically
non-zero, node_name is populated with garbage. This causes the target
lookup to fail because it attempts to match the garbage value:
if (node_name && cpu_to_be64(tgt->ids.node_name) != node_name)
continue;
Could this skip valid targets and unintentionally discard the events?
> + if ((subq->flags & IBMVFC_ASYNC_IS_FPIN_EXT) == 0) {
> + fpin = ibmvfc_full_fpin_to_desc(subq);
> + } else if (!(sqfpin->fpin_data.flags & IBMVFC_FPIN_EVENT_TYPE_VALID)) {
> + dev_err_ratelimited(vhost->dev,
[ ... ]
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc_kunit.c b/drivers/scsi/ibmvscsi/ibmvfc_kunit.c
> index 86c1f6daca586..2f43916b43a5a 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc_kunit.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc_kunit.c
[ ... ]
> @@ -62,8 +63,6 @@ static void ibmvfc_async_fpin_test(struct kunit *test)
> msleep(1U);
> }
>
> - msleep(500U);
[Severity: Medium]
Is a 1ms delay sufficient to prevent racy tests?
The test calls ibmvfc_handle_async(), which queues work on vhost->fpin_workq.
Relying on msleep(1U) to wait for the worker thread to finish before reading
updated statistics is inherently racy. If the work is not scheduled within 1ms
under heavy load, the test may read stale values and fail.
Should this deterministically synchronize by calling flush_workqueue() instead?
> post[IBMVFC_AE_FPIN_LINK_CONGESTED] = READ_ONCE(fc_host->fpin_stats.cn_device_specific);
> post[IBMVFC_AE_FPIN_PORT_CONGESTED] = READ_ONCE(tgt->rport->fpin_stats.cn);
> post[IBMVFC_AE_FPIN_PORT_CLEARED] = READ_ONCE(tgt->rport->fpin_stats.cn_clear);
[ ... ]
> @@ -116,8 +115,117 @@ static void ibmvfc_async_fpin_test(struct kunit *test)
> post[IBMVFC_AE_FPIN_CONGESTION_CLEARED]);
> }
>
> +#define IBMVFC_TEST_FPIN_EXT(fs, ev, stat, crq) { \
> + crq.valid = 0x80; \
> + crq.flags = IBMVFC_ASYNC_IS_FPIN_EXT; \
> + crq.link_state = IBMVFC_AE_LS_LINK_UP; \
> + crq.fpin_status = (fs); \
> + crq.event = cpu_to_be16(IBMVFC_AE_FPIN); \
> + crq.wwpn = cpu_to_be64(tgt->wwpn); \
> + crq.fpin_data.flags = IBMVFC_FPIN_EVENT_TYPE_VALID; \
> + crq.fpin_data.event_type = cpu_to_be16((ev)); \
> + pre = READ_ONCE(tgt->rport->fpin_stats.stat); \
> + ibmvfc_handle_async((struct ibmvfc_crq *)&crq, vhost, true); \
> + msleep(1U); \
> + post = READ_ONCE(tgt->rport->fpin_stats.stat); \
> +}
[Severity: Medium]
Are there missing test assertions here?
The IBMVFC_TEST_FPIN_EXT macro assigns pre and post statistic values but lacks
any KUNIT_EXPECT_* assertions to actually verify the outcomes.
Additionally, because the dummy crq structures are zero-initialized in
ibmvfc_extended_fpin_test(), event_threshold and event_count are 0. This
coincidentally masks the overlapping node_name extraction bug mentioned
earlier because node_name evaluates to 0, which bypasses the broken target
lookup check and allows the test to artificially succeed.
Could assertions be added, and non-zero dummy data be used to thoroughly test
this path?
Also, similar to ibmvfc_async_fpin_test(), the msleep(1U) used here might
lead to flaky test failures if the workqueue runs slower than expected.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-ibmvfc-fpin-support-v4-0-ef031ac19520@linux.ibm.com?part=8
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2026-07-10 19:40 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-10 19:16 [PATCH v4 0/8] ibmvfc: make ibmvfc support FPIN messages Dave Marquardt via B4 Relay
2026-07-10 19:16 ` [PATCH v4 1/8] ibmvfc: add basic FPIN support Dave Marquardt via B4 Relay
2026-07-10 19:31 ` sashiko-bot
2026-07-10 19:16 ` [PATCH v4 2/8] ibmvfc: Add NOOP command support Dave Marquardt via B4 Relay
2026-07-10 19:31 ` sashiko-bot
2026-07-10 19:16 ` [PATCH v4 3/8] ibmvfc: make ibmvfc login to fabric Dave Marquardt via B4 Relay
2026-07-10 19:30 ` sashiko-bot
2026-07-10 19:16 ` [PATCH v4 4/8] ibmvfc: define asynchronous sub-queue Dave Marquardt via B4 Relay
2026-07-10 19:40 ` sashiko-bot
2026-07-10 19:16 ` [PATCH v4 5/8] ibmvfc: allocate " Dave Marquardt via B4 Relay
2026-07-10 19:36 ` sashiko-bot
2026-07-10 19:16 ` [PATCH v4 6/8] ibmvfc: extend async event handlers to handle async sub queue events Dave Marquardt via B4 Relay
2026-07-10 19:37 ` sashiko-bot
2026-07-10 19:16 ` [PATCH v4 7/8] ibmvfc: register and use asynchronous sub-queue for events Dave Marquardt via B4 Relay
2026-07-10 19:32 ` sashiko-bot
2026-07-10 19:16 ` [PATCH v4 8/8] ibmvfc: handle extended FPIN events Dave Marquardt via B4 Relay
2026-07-10 19:40 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox