* [PATCH v16 01/12] scsi: ibmvfc: fix IRQ leak and guard deregister on channel reg failure
2026-10-26 16:12 [PATCH v16 00/12] scsi: ibmvfc: make ibmvfc support FPIN messages Dave Marquardt via B4 Relay
@ 2026-10-26 16:12 ` Dave Marquardt via B4 Relay
2026-10-26 16:12 ` [PATCH v16 02/12] scsi: ibmvfc: fix potential clobbering of rc after failed irq setup Dave Marquardt via B4 Relay
` (10 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Dave Marquardt via B4 Relay @ 2026-10-26 16:12 UTC (permalink / raw)
To: Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy (CS GROUP), James E.J. Bottomley,
Martin K. Petersen, Tyrel Datwyler
Cc: linux-scsi, linuxppc-dev, linux-kernel, Dave Marquardt
From: Dave Marquardt <davemarq@linux.ibm.com>
If request_irq() fails during ibmvfc_register_channel(), the error path
disposes of the IRQ mapping via irq_dispose_mapping() but leaves scrq->irq
populated with the stale virq number. A subsequent call to
ibmvfc_deregister_channel() during teardown or reset cleanup then
unconditionally invokes free_irq() and irq_dispose_mapping() on the stale
IRQ descriptor, triggering kernel warnings.
Fix this by:
1. Explicitly setting scrq->irq to 0 after irq_dispose_mapping() in the
ibmvfc_register_channel() error path.
2. Guarding free_irq() and irq_dispose_mapping() with a check for scrq->irq
in ibmvfc_deregister_channel() so unmapped or unregistered queues are
safely skipped.
Signed-off-by: Dave Marquardt <davemarq@linux.ibm.com>
Acked-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc-core.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
index 0b154e07299c..7818838669ae 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
@@ -6435,6 +6435,7 @@ static int ibmvfc_register_channel(struct ibmvfc_host *vhost,
if (rc) {
dev_err(dev, "Couldn't register sub-crq[%d] irq\n", index);
irq_dispose_mapping(scrq->irq);
+ scrq->irq = 0;
goto irq_failed;
}
@@ -6463,9 +6464,11 @@ static void ibmvfc_deregister_channel(struct ibmvfc_host *vhost,
ENTER;
- free_irq(scrq->irq, scrq);
- irq_dispose_mapping(scrq->irq);
- scrq->irq = 0;
+ if (scrq->irq) {
+ free_irq(scrq->irq, scrq);
+ irq_dispose_mapping(scrq->irq);
+ scrq->irq = 0;
+ }
do {
rc = plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address,
--
2.56.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v16 02/12] scsi: ibmvfc: fix potential clobbering of rc after failed irq setup
2026-10-26 16:12 [PATCH v16 00/12] scsi: ibmvfc: make ibmvfc support FPIN messages Dave Marquardt via B4 Relay
2026-10-26 16:12 ` [PATCH v16 01/12] scsi: ibmvfc: fix IRQ leak and guard deregister on channel reg failure Dave Marquardt via B4 Relay
@ 2026-10-26 16:12 ` Dave Marquardt via B4 Relay
2026-10-26 16:12 ` [PATCH v16 03/12] scsi: ibmvfc: fix potential NULL pointer dereference on failed queue allocation Dave Marquardt via B4 Relay
` (9 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Dave Marquardt via B4 Relay @ 2026-10-26 16:12 UTC (permalink / raw)
To: Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy (CS GROUP), James E.J. Bottomley,
Martin K. Petersen, Tyrel Datwyler
Cc: linux-scsi, linuxppc-dev, linux-kernel, Dave Marquardt
From: Tyrel Datwyler <tyreld@linux.ibm.com>
Fix a variable-shadowing bug in ibmvfc_register_channel() where the
irq_failed cleanup loop reused rc for the H_FREE_SUB_CRQ hcall result,
clobbering the error code returned to the caller.
Introduce hcall_rc for the cleanup loop instead.
Signed-off-by: Dave Marquardt <davemarq@linux.ibm.com>
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc-core.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
index 7818838669ae..145d559041f1 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
@@ -6391,6 +6391,7 @@ static int ibmvfc_register_channel(struct ibmvfc_host *vhost,
struct vio_dev *vdev = to_vio_dev(dev);
struct ibmvfc_queue *scrq = &channels->scrqs[index];
int rc = -ENOMEM;
+ int hcall_rc;
ENTER;
@@ -6446,8 +6447,8 @@ static int ibmvfc_register_channel(struct ibmvfc_host *vhost,
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;
--
2.56.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v16 03/12] scsi: ibmvfc: fix potential NULL pointer dereference on failed queue allocation
2026-10-26 16:12 [PATCH v16 00/12] scsi: ibmvfc: make ibmvfc support FPIN messages Dave Marquardt via B4 Relay
2026-10-26 16:12 ` [PATCH v16 01/12] scsi: ibmvfc: fix IRQ leak and guard deregister on channel reg failure Dave Marquardt via B4 Relay
2026-10-26 16:12 ` [PATCH v16 02/12] scsi: ibmvfc: fix potential clobbering of rc after failed irq setup Dave Marquardt via B4 Relay
@ 2026-10-26 16:12 ` Dave Marquardt via B4 Relay
2026-10-26 16:26 ` sashiko-bot
2026-10-26 16:12 ` [PATCH v16 04/12] scsi: ibmvfc: fix unguarded dma_unmap_single() in ibmvfc_free_queue() Dave Marquardt via B4 Relay
` (8 subsequent siblings)
11 siblings, 1 reply; 21+ messages in thread
From: Dave Marquardt via B4 Relay @ 2026-10-26 16:12 UTC (permalink / raw)
To: Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy (CS GROUP), James E.J. Bottomley,
Martin K. Petersen, Tyrel Datwyler
Cc: linux-scsi, linuxppc-dev, linux-kernel, Dave Marquardt
From: Tyrel Datwyler <tyreld@linux.ibm.com>
Guard ibmvfc_deregister_channel() against calling H_FREE_SUB_CRQ on a
queue that was never registered and against memsetting a buffer that was
never allocated.
Add an scrq->cookie guard around the H_FREE_SUB_CRQ hypercall so it is
only issued when the cookie is non-zero; clear scrq->cookie to zero on
success to prevent double-free if the function is called again during
reset or teardown. Add scrq->cookie = 0 in the ibmvfc_register_channel()
irq_failed: cleanup path for the same reason. Guard the memset() of the
queue message buffer behind a check of scrq->msgs.handle to prevent a
NULL dereference when the function is called on a queue that was never
allocated, such as async_sub_crq when multi-queue is disabled or
initialization failed before ibmvfc_alloc_queue() was reached. Update the
error message to include channels->protocol so failures can be
distinguished across SCSI, NVMe, and async sub-CRQ channels.
Expose ibmvfc_deregister_channel() via VISIBLE_IF_KUNIT and
EXPORT_SYMBOL_IF_KUNIT and add a kernel-doc header to support KUnit
testing.
Add ibmvfc_get_first_vhost() to retrieve the first registered ibmvfc
adapter with a reference held; expose it via VISIBLE_IF_KUNIT and
EXPORT_SYMBOL_IF_KUNIT for use by KUnit tests.
Add SCSI_IBMVFC_KUNIT_TEST Kconfig entry and Makefile line, and create
ibmvfc_kunit.c with an ibmvfc-channel-test suite containing
ibmvfc_deregister_channel_guard_test(). The test verifies that calling
ibmvfc_deregister_channel() on a queue with cookie == 0 and
msgs.handle == NULL issues no hypercall, performs no memset, and leaves
the queue state unchanged.
Signed-off-by: Dave Marquardt <davemarq@linux.ibm.com>
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/Kconfig | 10 ++++++
drivers/scsi/ibmvscsi/Makefile | 1 +
drivers/scsi/ibmvscsi/ibmvfc-core.c | 64 +++++++++++++++++++++++++++++++-----
drivers/scsi/ibmvscsi/ibmvfc.h | 8 +++++
drivers/scsi/ibmvscsi/ibmvfc_kunit.c | 63 +++++++++++++++++++++++++++++++++++
5 files changed, 137 insertions(+), 9 deletions(-)
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index 4a2af0f702e1..f47789b1102d 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -759,6 +759,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
+ cover the sub-CRQ channel deregistration guard path.
+ 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 9408c7f4cdee..a227bc633f3a 100644
--- a/drivers/scsi/ibmvscsi/Makefile
+++ b/drivers/scsi/ibmvscsi/Makefile
@@ -3,3 +3,4 @@ ibmvfc-objs := ibmvfc-core.o ibmvfc-nvme.o
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-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
index 145d559041f1..f0502d748210 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
@@ -6450,11 +6450,23 @@ static int ibmvfc_register_channel(struct ibmvfc_host *vhost,
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:
+ scrq->cookie = 0;
LEAVE;
return rc;
}
-static void ibmvfc_deregister_channel(struct ibmvfc_host *vhost,
+/**
+ * ibmvfc_deregister_channel - Deregister a sub-CRQ channel with the hypervisor
+ * @vhost: ibmvfc host struct
+ * @channels: ibmvfc channels struct containing the sub-CRQ array
+ * @index: index into the sub-CRQ array
+ *
+ * Frees the IRQ, disposes of the IRQ mapping, and calls H_FREE_SUB_CRQ to
+ * release the sub-CRQ with the hypervisor. On success the queue message
+ * buffer is zeroed and the current index is reset. If H_FREE_SUB_CRQ fails,
+ * an error is logged but the channel resources are cleaned up regardless.
+ */
+VISIBLE_IF_KUNIT void ibmvfc_deregister_channel(struct ibmvfc_host *vhost,
struct ibmvfc_channels *channels,
int index)
{
@@ -6471,20 +6483,28 @@ static void ibmvfc_deregister_channel(struct ibmvfc_host *vhost,
scrq->irq = 0;
}
- do {
- rc = plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address,
- scrq->cookie);
- } while (rc == H_BUSY || H_IS_LONG_BUSY(rc));
+ if (scrq->cookie) {
+ do {
+ rc = plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address,
+ scrq->cookie);
+ } 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);
+ if (rc)
+ dev_err(dev, "Failed to free protocol (%d) sub-crq[%d]: rc=%ld\n",
+ channels->protocol, index, rc);
+ else
+ scrq->cookie = 0;
+ }
/* Clean out the queue */
- memset(scrq->msgs.crq, 0, PAGE_SIZE);
- scrq->cur = 0;
+ if (scrq->msgs.handle) {
+ memset(scrq->msgs.crq, 0, PAGE_SIZE);
+ scrq->cur = 0;
+ }
LEAVE;
}
+EXPORT_SYMBOL_IF_KUNIT(ibmvfc_deregister_channel);
static void ibmvfc_reg_sub_crqs(struct ibmvfc_host *vhost,
struct ibmvfc_channels *channels)
@@ -7118,5 +7138,31 @@ static void __exit ibmvfc_module_exit(void)
fc_release_transport(ibmvfc_transport_template);
}
+#if IS_ENABLED(CONFIG_KUNIT)
+/**
+ * ibmvfc_get_first_vhost - Get the first ibmvfc host from the global list
+ *
+ * Grabs the driver spinlock and retrieves the first host adapter from the
+ * driver list. If found, increments its reference count via scsi_host_get().
+ *
+ * Return value:
+ * struct ibmvfc_host * with reference held on success, NULL otherwise
+ **/
+VISIBLE_IF_KUNIT struct ibmvfc_host *ibmvfc_get_first_vhost(void)
+{
+ struct ibmvfc_host *vhost = NULL;
+
+ spin_lock(&ibmvfc_driver_lock);
+ if (!list_empty(&ibmvfc_head))
+ vhost = list_first_entry(&ibmvfc_head, struct ibmvfc_host, queue);
+ if (vhost)
+ if (!scsi_host_get(vhost->host))
+ vhost = NULL;
+ spin_unlock(&ibmvfc_driver_lock);
+ return vhost;
+}
+EXPORT_SYMBOL_IF_KUNIT(ibmvfc_get_first_vhost);
+#endif
+
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 ca80ceffe53a..421ce8bc3ea3 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.h
+++ b/drivers/scsi/ibmvscsi/ibmvfc.h
@@ -1088,4 +1088,12 @@ static inline struct ibmvfc_host *ibmvfc_channels_to_vhost(struct ibmvfc_channel
#define ibmvfc_remove_trace_file(kobj, attr) do { } while (0)
#endif
+#if IS_ENABLED(CONFIG_KUNIT)
+#include <kunit/visibility.h>
+VISIBLE_IF_KUNIT void ibmvfc_deregister_channel(struct ibmvfc_host *vhost,
+ struct ibmvfc_channels *channels,
+ int index);
+VISIBLE_IF_KUNIT struct ibmvfc_host *ibmvfc_get_first_vhost(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..24799aa2368a
--- /dev/null
+++ b/drivers/scsi/ibmvscsi/ibmvfc_kunit.c
@@ -0,0 +1,63 @@
+// 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 "ibmvfc.h"
+
+MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
+
+/**
+ * ibmvfc_deregister_channel_guard_test - unit test for ibmvfc_deregister_channel()
+ * guard conditions
+ * @test: pointer to kunit structure
+ *
+ * Tests that ibmvfc_deregister_channel() safely handles a queue that was
+ * never registered with the hypervisor (cookie == 0, msgs.handle == NULL):
+ * - no H_FREE_SUB_CRQ hypercall is issued
+ * - no memset is attempted on a NULL buffer
+ * - cookie and cur remain zero after the call
+ *
+ * Return: void
+ */
+static void ibmvfc_deregister_channel_guard_test(struct kunit *test)
+{
+ struct ibmvfc_channels channels = {};
+ struct ibmvfc_queue scrq = {};
+ struct ibmvfc_host *vhost;
+
+ vhost = ibmvfc_get_first_vhost();
+ if (!vhost)
+ kunit_skip(test, "No ibmvfc devices available");
+
+ /*
+ * Embed the synthetic queue in a channels struct with one entry.
+ * cookie == 0 and msgs.handle == NULL: ibmvfc_deregister_channel()
+ * must skip both the H_FREE_SUB_CRQ hypercall and the memset.
+ */
+ channels.scrqs = &scrq;
+ channels.max_queues = 1;
+
+ ibmvfc_deregister_channel(vhost, &channels, 0);
+
+ KUNIT_EXPECT_EQ(test, scrq.cookie, 0UL);
+ KUNIT_EXPECT_EQ(test, scrq.cur, 0);
+ KUNIT_EXPECT_NULL(test, scrq.msgs.handle);
+
+ scsi_host_put(vhost->host);
+}
+
+static struct kunit_case ibmvfc_channel_test_cases[] = {
+ KUNIT_CASE(ibmvfc_deregister_channel_guard_test),
+ {},
+};
+
+static struct kunit_suite ibmvfc_channel_test_suite = {
+ .name = "ibmvfc-channel-test",
+ .test_cases = ibmvfc_channel_test_cases,
+};
+kunit_test_init_section_suite(ibmvfc_channel_test_suite);
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Dave Marquardt <davemarq@linux.ibm.com>");
+MODULE_DESCRIPTION("Test module for IBM Virtual Fibre Channel Driver");
--
2.56.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v16 03/12] scsi: ibmvfc: fix potential NULL pointer dereference on failed queue allocation
2026-10-26 16:12 ` [PATCH v16 03/12] scsi: ibmvfc: fix potential NULL pointer dereference on failed queue allocation Dave Marquardt via B4 Relay
@ 2026-10-26 16:26 ` sashiko-bot
2026-10-26 21:17 ` Dave Marquardt
0 siblings, 1 reply; 21+ messages in thread
From: sashiko-bot @ 2026-10-26 16:26 UTC (permalink / raw)
To: Dave Marquardt; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] scsi: ibmvfc: build failure when CONFIG_KUNIT is disabled
--
commit e6767e190b474e7d3a6e33d2ed96de8dbb05ff1a
Author: Tyrel Datwyler <tyreld@linux.ibm.com>
scsi: ibmvfc: fix potential NULL pointer dereference on failed queue allocation
This commit guards ibmvfc_deregister_channel() against freeing an
unregistered sub-CRQ or clearing an unallocated message buffer, and adds a
KUnit test suite to verify the guard conditions.
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
> index 145d559041f12..f0502d7482105 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
> @@ -6450,11 +6450,23 @@ static int ibmvfc_register_channel(struct ibmvfc_host *vhost,
> 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:
> + scrq->cookie = 0;
> LEAVE;
> return rc;
> }
>
> -static void ibmvfc_deregister_channel(struct ibmvfc_host *vhost,
> +/**
> + * ibmvfc_deregister_channel - Deregister a sub-CRQ channel with the hypervisor
> + * @vhost: ibmvfc host struct
> + * @channels: ibmvfc channels struct containing the sub-CRQ array
> + * @index: index into the sub-CRQ array
> + *
> + * Frees the IRQ, disposes of the IRQ mapping, and calls H_FREE_SUB_CRQ to
> + * release the sub-CRQ with the hypervisor. On success the queue message
> + * buffer is zeroed and the current index is reset. If H_FREE_SUB_CRQ fails,
> + * an error is logged but the channel resources are cleaned up regardless.
> + */
> +VISIBLE_IF_KUNIT void ibmvfc_deregister_channel(struct ibmvfc_host *vhost,
> struct ibmvfc_channels *channels,
> int index)
> {
> @@ -6471,20 +6483,28 @@ static void ibmvfc_deregister_channel(struct ibmvfc_host *vhost,
> scrq->irq = 0;
> }
>
> - do {
> - rc = plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address,
> - scrq->cookie);
> - } while (rc == H_BUSY || H_IS_LONG_BUSY(rc));
> + if (scrq->cookie) {
> + do {
> + rc = plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address,
> + scrq->cookie);
> + } 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);
> + if (rc)
> + dev_err(dev, "Failed to free protocol (%d) sub-crq[%d]: rc=%ld\n",
> + channels->protocol, index, rc);
> + else
> + scrq->cookie = 0;
> + }
>
> /* Clean out the queue */
> - memset(scrq->msgs.crq, 0, PAGE_SIZE);
> - scrq->cur = 0;
> + if (scrq->msgs.handle) {
> + memset(scrq->msgs.crq, 0, PAGE_SIZE);
> + scrq->cur = 0;
> + }
>
> LEAVE;
> }
> +EXPORT_SYMBOL_IF_KUNIT(ibmvfc_deregister_channel);
[ ... ]
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.h b/drivers/scsi/ibmvscsi/ibmvfc.h
> index ca80ceffe53a5..421ce8bc3ea33 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc.h
> +++ b/drivers/scsi/ibmvscsi/ibmvfc.h
> @@ -1088,4 +1088,12 @@ static inline struct ibmvfc_host *ibmvfc_channels_to_vhost(struct ibmvfc_channel
> #define ibmvfc_remove_trace_file(kobj, attr) do { } while (0)
> #endif
>
> +#if IS_ENABLED(CONFIG_KUNIT)
> +#include <kunit/visibility.h>
> +VISIBLE_IF_KUNIT void ibmvfc_deregister_channel(struct ibmvfc_host *vhost,
> + struct ibmvfc_channels *channels,
> + int index);
> +VISIBLE_IF_KUNIT struct ibmvfc_host *ibmvfc_get_first_vhost(void);
> +#endif
[Severity: Low]
Does this break the build when CONFIG_SCSI_IBMVFC is enabled and CONFIG_KUNIT
is disabled?
In drivers/scsi/ibmvscsi/ibmvfc-core.c, ibmvfc_deregister_channel() uses
VISIBLE_IF_KUNIT and EXPORT_SYMBOL_IF_KUNIT(ibmvfc_deregister_channel)
outside of any CONFIG_KUNIT guard.
However, drivers/scsi/ibmvscsi/ibmvfc.h only includes <kunit/visibility.h>
when CONFIG_KUNIT is enabled, so the fallback definitions in
include/kunit/visibility.h:
#else
#define VISIBLE_IF_KUNIT static
#define EXPORT_SYMBOL_IF_KUNIT(symbol)
#endif
are never pulled in when CONFIG_KUNIT is disabled, causing compilation of
drivers/scsi/ibmvscsi/ibmvfc-core.c to fail due to unknown macros.
A later commit in the series, commit 1bc41de0e60f ("scsi: ibmvfc: add basic
FPIN support"), adds an unconditional #include <kunit/visibility.h> to
drivers/scsi/ibmvscsi/ibmvfc-core.c, though leaving this commit and commit
1b37c48a1902 ("scsi: ibmvfc: add link state to host struct") unbuildable with
CONFIG_KUNIT=n during bisection.
Should <kunit/visibility.h> be included unconditionally in
drivers/scsi/ibmvscsi/ibmvfc.h (or drivers/scsi/ibmvscsi/ibmvfc-core.c) in
this commit?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261026-ibmvfc-fpin-support-v16-0-a900aa657ed5@linux.ibm.com?part=3
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v16 03/12] scsi: ibmvfc: fix potential NULL pointer dereference on failed queue allocation
2026-10-26 16:26 ` sashiko-bot
@ 2026-10-26 21:17 ` Dave Marquardt
0 siblings, 0 replies; 21+ messages in thread
From: Dave Marquardt @ 2026-10-26 21:17 UTC (permalink / raw)
To: sashiko-bot; +Cc: sashiko-reviews, linux-scsi
sashiko-bot@kernel.org writes:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] scsi: ibmvfc: build failure when CONFIG_KUNIT is disabled
> --
>
> commit e6767e190b474e7d3a6e33d2ed96de8dbb05ff1a
> Author: Tyrel Datwyler <tyreld@linux.ibm.com>
>
> scsi: ibmvfc: fix potential NULL pointer dereference on failed queue allocation
>
> This commit guards ibmvfc_deregister_channel() against freeing an
> unregistered sub-CRQ or clearing an unallocated message buffer, and adds a
> KUnit test suite to verify the guard conditions.
>
>> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
>> index 145d559041f12..f0502d7482105 100644
>> --- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
>> +++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
>> @@ -6450,11 +6450,23 @@ static int ibmvfc_register_channel(struct ibmvfc_host *vhost,
>> 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:
>> + scrq->cookie = 0;
>> LEAVE;
>> return rc;
>> }
>>
>> -static void ibmvfc_deregister_channel(struct ibmvfc_host *vhost,
>> +/**
>> + * ibmvfc_deregister_channel - Deregister a sub-CRQ channel with the hypervisor
>> + * @vhost: ibmvfc host struct
>> + * @channels: ibmvfc channels struct containing the sub-CRQ array
>> + * @index: index into the sub-CRQ array
>> + *
>> + * Frees the IRQ, disposes of the IRQ mapping, and calls H_FREE_SUB_CRQ to
>> + * release the sub-CRQ with the hypervisor. On success the queue message
>> + * buffer is zeroed and the current index is reset. If H_FREE_SUB_CRQ fails,
>> + * an error is logged but the channel resources are cleaned up regardless.
>> + */
>> +VISIBLE_IF_KUNIT void ibmvfc_deregister_channel(struct ibmvfc_host *vhost,
>> struct ibmvfc_channels *channels,
>> int index)
>> {
>> @@ -6471,20 +6483,28 @@ static void ibmvfc_deregister_channel(struct ibmvfc_host *vhost,
>> scrq->irq = 0;
>> }
>>
>> - do {
>> - rc = plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address,
>> - scrq->cookie);
>> - } while (rc == H_BUSY || H_IS_LONG_BUSY(rc));
>> + if (scrq->cookie) {
>> + do {
>> + rc = plpar_hcall_norets(H_FREE_SUB_CRQ, vdev->unit_address,
>> + scrq->cookie);
>> + } 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);
>> + if (rc)
>> + dev_err(dev, "Failed to free protocol (%d) sub-crq[%d]: rc=%ld\n",
>> + channels->protocol, index, rc);
>> + else
>> + scrq->cookie = 0;
>> + }
>>
>> /* Clean out the queue */
>> - memset(scrq->msgs.crq, 0, PAGE_SIZE);
>> - scrq->cur = 0;
>> + if (scrq->msgs.handle) {
>> + memset(scrq->msgs.crq, 0, PAGE_SIZE);
>> + scrq->cur = 0;
>> + }
>>
>> LEAVE;
>> }
>> +EXPORT_SYMBOL_IF_KUNIT(ibmvfc_deregister_channel);
>
> [ ... ]
>
>> diff --git a/drivers/scsi/ibmvscsi/ibmvfc.h b/drivers/scsi/ibmvscsi/ibmvfc.h
>> index ca80ceffe53a5..421ce8bc3ea33 100644
>> --- a/drivers/scsi/ibmvscsi/ibmvfc.h
>> +++ b/drivers/scsi/ibmvscsi/ibmvfc.h
>> @@ -1088,4 +1088,12 @@ static inline struct ibmvfc_host *ibmvfc_channels_to_vhost(struct ibmvfc_channel
>> #define ibmvfc_remove_trace_file(kobj, attr) do { } while (0)
>> #endif
>>
>> +#if IS_ENABLED(CONFIG_KUNIT)
>> +#include <kunit/visibility.h>
>> +VISIBLE_IF_KUNIT void ibmvfc_deregister_channel(struct ibmvfc_host *vhost,
>> + struct ibmvfc_channels *channels,
>> + int index);
>> +VISIBLE_IF_KUNIT struct ibmvfc_host *ibmvfc_get_first_vhost(void);
>> +#endif
>
> [Severity: Low]
> Does this break the build when CONFIG_SCSI_IBMVFC is enabled and CONFIG_KUNIT
> is disabled?
>
> In drivers/scsi/ibmvscsi/ibmvfc-core.c, ibmvfc_deregister_channel() uses
> VISIBLE_IF_KUNIT and EXPORT_SYMBOL_IF_KUNIT(ibmvfc_deregister_channel)
> outside of any CONFIG_KUNIT guard.
>
> However, drivers/scsi/ibmvscsi/ibmvfc.h only includes <kunit/visibility.h>
> when CONFIG_KUNIT is enabled, so the fallback definitions in
> include/kunit/visibility.h:
>
> #else
> #define VISIBLE_IF_KUNIT static
> #define EXPORT_SYMBOL_IF_KUNIT(symbol)
> #endif
>
> are never pulled in when CONFIG_KUNIT is disabled, causing compilation of
> drivers/scsi/ibmvscsi/ibmvfc-core.c to fail due to unknown macros.
>
> A later commit in the series, commit 1bc41de0e60f ("scsi: ibmvfc: add basic
> FPIN support"), adds an unconditional #include <kunit/visibility.h> to
> drivers/scsi/ibmvscsi/ibmvfc-core.c, though leaving this commit and commit
> 1b37c48a1902 ("scsi: ibmvfc: add link state to host struct") unbuildable with
> CONFIG_KUNIT=n during bisection.
>
> Should <kunit/visibility.h> be included unconditionally in
> drivers/scsi/ibmvscsi/ibmvfc.h (or drivers/scsi/ibmvscsi/ibmvfc-core.c) in
> this commit?
I'm configured a kernel build pseries_le_defconfig and then turned
CONFIG_KUNIT off. Indeed, there's an error when building ibmvfc-core.c.
I tried including <kunit/visibility.h> unconditionally in ibmvfc.h, but
that caused build warnings for ibmvfc-nvme.c about unused
functions. Including <kunit/visibility.h> in ibmvfc-core.c resolved the
issue.
This will be the fix in v17.
-Dave
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v16 04/12] scsi: ibmvfc: fix unguarded dma_unmap_single() in ibmvfc_free_queue()
2026-10-26 16:12 [PATCH v16 00/12] scsi: ibmvfc: make ibmvfc support FPIN messages Dave Marquardt via B4 Relay
` (2 preceding siblings ...)
2026-10-26 16:12 ` [PATCH v16 03/12] scsi: ibmvfc: fix potential NULL pointer dereference on failed queue allocation Dave Marquardt via B4 Relay
@ 2026-10-26 16:12 ` Dave Marquardt via B4 Relay
2026-10-26 16:12 ` [PATCH v16 05/12] scsi: ibmvfc: add basic FPIN support Dave Marquardt via B4 Relay
` (7 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Dave Marquardt via B4 Relay @ 2026-10-26 16:12 UTC (permalink / raw)
To: Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy (CS GROUP), James E.J. Bottomley,
Martin K. Petersen, Tyrel Datwyler
Cc: linux-scsi, linuxppc-dev, linux-kernel, Dave Marquardt
From: Dave Marquardt <davemarq@linux.ibm.com>
ibmvfc_free_queue() unconditionally calls dma_unmap_single() before
freeing the queue page, but only guards free_page() behind a
queue->msgs.handle check. If ibmvfc_alloc_queue() fails before the
DMA mapping is established (e.g. get_zeroed_page() fails or
ibmvfc_init_event_pool() fails), msgs.handle and msg_token are both
zero/NULL. Calling dma_unmap_single() with address 0 is unsafe on
many architectures.
Guard dma_unmap_single() and free_page() together under the existing
msgs.handle check so ibmvfc_free_queue() is safe to call on a
partially or never-initialized queue.
Signed-off-by: Dave Marquardt <davemarq@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc-core.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
index f0502d748210..b63b341efb07 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
@@ -913,9 +913,11 @@ static void ibmvfc_free_queue(struct ibmvfc_host *vhost,
{
struct device *dev = vhost->dev;
- dma_unmap_single(dev, queue->msg_token, PAGE_SIZE, DMA_BIDIRECTIONAL);
- free_page((unsigned long)queue->msgs.handle);
- queue->msgs.handle = NULL;
+ if (queue->msgs.handle) {
+ dma_unmap_single(dev, queue->msg_token, PAGE_SIZE, DMA_BIDIRECTIONAL);
+ free_page((unsigned long)queue->msgs.handle);
+ queue->msgs.handle = NULL;
+ }
ibmvfc_free_event_pool(vhost, queue);
}
--
2.56.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v16 05/12] scsi: ibmvfc: add basic FPIN support
2026-10-26 16:12 [PATCH v16 00/12] scsi: ibmvfc: make ibmvfc support FPIN messages Dave Marquardt via B4 Relay
` (3 preceding siblings ...)
2026-10-26 16:12 ` [PATCH v16 04/12] scsi: ibmvfc: fix unguarded dma_unmap_single() in ibmvfc_free_queue() Dave Marquardt via B4 Relay
@ 2026-10-26 16:12 ` Dave Marquardt via B4 Relay
2026-10-26 16:24 ` sashiko-bot
2026-10-26 16:12 ` [PATCH v16 06/12] scsi: ibmvfc: add NOOP command support Dave Marquardt via B4 Relay
` (6 subsequent siblings)
11 siblings, 1 reply; 21+ messages in thread
From: Dave Marquardt via B4 Relay @ 2026-10-26 16:12 UTC (permalink / raw)
To: Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy (CS GROUP), James E.J. Bottomley,
Martin K. Petersen, Tyrel Datwyler
Cc: linux-scsi, linuxppc-dev, linux-kernel, 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 via a per-adapter ordered workqueue (fpin_workq). 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.
The per-adapter workqueue is allocated in ibmvfc_probe() and destroyed
in ibmvfc_remove() after ibmvfc_release_crq_queue(). This ordering is
critical: ibmvfc_release_crq_queue() disables CRQ interrupts and kills
the tasklet, ensuring no new work items can be queued after that point.
destroy_workqueue() then drains any already-queued items before
ibmvfc_free_mem() runs, preventing use-after-free of tgt_pool.
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.
The KUnit test uses a private owned workqueue per test created with
alloc_ordered_workqueue(). This gives ibmvfc_process_async_work() its
own thread stack, avoiding kernel stack overflow when called from the
deep KUnit test context.
Add ibmvfc_handle_async() support for IBMVFC_AE_FPIN events, FPIN
message conversion to fc_els_fpin format, handling of link congestion,
port congestion, port cleared, port degraded, and congestion cleared
events, and a KUnit test module for FPIN functionality.
Signed-off-by: Dave Marquardt <davemarq@linux.ibm.com>
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/Kconfig | 6 +-
drivers/scsi/ibmvscsi/ibmvfc-core.c | 287 ++++++++++++++++++++++++++++++++++-
drivers/scsi/ibmvscsi/ibmvfc.h | 12 ++
drivers/scsi/ibmvscsi/ibmvfc_kunit.c | 183 ++++++++++++++++++++++
4 files changed, 481 insertions(+), 7 deletions(-)
diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index f47789b1102d..db934a18488d 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -765,9 +765,9 @@ config SCSI_IBMVFC_KUNIT_TEST
default KUNIT_ALL_TESTS
help
Compile IBM POWER Virtual FC client KUnit tests. These tests
- cover the sub-CRQ channel deregistration guard path.
- To compile this driver as a module, choose M here: the module
- will be called ibmvfc_kunit.
+ cover the sub-CRQ channel deregistration guard path and FPIN
+ event processing. 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"
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
index b63b341efb07..1cd39d36edf5 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
@@ -31,6 +31,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;
@@ -1435,6 +1438,7 @@ void ibmvfc_release_tgt(struct kref *kref)
struct ibmvfc_target *tgt = container_of(kref, struct ibmvfc_target, kref);
mempool_free(tgt, tgt->vhost->tgt_pool);
}
+EXPORT_SYMBOL_IF_KUNIT(ibmvfc_release_tgt);
/**
* ibmvfc_get_starget_node_name - Get SCSI target's node name
@@ -3215,6 +3219,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 = {
@@ -3263,6 +3268,255 @@ 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.
+ * @fpin_status: IBMVFC FPIN status value; selects descriptor type
+ * @wwpn: attached port WWPN (big-endian)
+ * @type: FC FPIN event type (big-endian)
+ * @modifier: FC FPIN event modifier (big-endian)
+ * @threshold: link integrity event threshold (big-endian); used for
+ * IBMVFC_AE_FPIN_PORT_DEGRADED only
+ * @event_count: link integrity event count (big-endian); used for
+ * IBMVFC_AE_FPIN_PORT_DEGRADED only
+ *
+ * Return:
+ * NULL - unable to allocate structure or unrecognised fpin_status
+ * 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_find_target - Search for a target in a target list
+ * @target_list: list head of targets to search
+ * @scsi_id: SCSI ID to match (0 to skip this check)
+ * @wwpn: WWPN to match (0 to skip this check)
+ * @node_name: Node name to match (0 to skip this check)
+ *
+ * Returns:
+ * Pointer to matching target, or NULL if not found
+ **/
+static struct ibmvfc_target *ibmvfc_find_target(struct list_head *target_list,
+ __be64 scsi_id, __be64 wwpn,
+ __be64 node_name)
+{
+ struct ibmvfc_target *tgt;
+
+ list_for_each_entry(tgt, target_list, queue) {
+ if (scsi_id && tgt->scsi_id != be64_to_cpu(scsi_id))
+ continue;
+ if (wwpn && tgt->ids.port_name != be64_to_cpu(wwpn))
+ continue;
+ if (node_name && tgt->ids.node_name != be64_to_cpu(node_name))
+ continue;
+ if (!tgt->rport && !tgt->nvme_remote_port)
+ continue;
+ return tgt;
+ }
+
+ return NULL;
+}
+
+/**
+ * ibmvfc_process_async_work - Process IBMVFC_AE_FPIN async CRQ from work queue
+ * @work: pointer to work_struct
+ */
+VISIBLE_IF_KUNIT void ibmvfc_process_async_work(struct work_struct *work)
+{
+ struct ibmvfc_async_work *aw;
+ struct ibmvfc_async_crq *crq;
+ struct ibmvfc_target *tgt;
+ struct ibmvfc_host *vhost;
+ struct fc_els_fpin *fpin;
+ unsigned long flags;
+
+ aw = container_of_const(work, struct ibmvfc_async_work, async_work_s);
+ vhost = aw->vhost;
+ crq = &aw->crq;
+
+ if (!crq->scsi_id && !crq->wwpn && !crq->node_name)
+ goto free;
+
+ spin_lock_irqsave(&vhost->host->host_lock, flags);
+ tgt = ibmvfc_find_target(&vhost->scsi_scrqs.targets, crq->scsi_id,
+ crq->wwpn, crq->node_name);
+ if (!tgt) {
+ /* Target not found in scsi_scrqs, search nvme_scrqs */
+ tgt = ibmvfc_find_target(&vhost->nvme_scrqs.targets,
+ crq->scsi_id, crq->wwpn,
+ crq->node_name);
+ }
+
+ if (tgt) {
+ 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;
+ }
+
+ 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 received, unable to process\n");
+
+ kref_put(&tgt->kref, ibmvfc_release_tgt);
+ free:
+ scsi_host_put(aw->vhost->host);
+ kfree(aw);
+}
+EXPORT_SYMBOL_IF_KUNIT(ibmvfc_process_async_work);
+
/**
* ibmvfc_handle_async - Handle an async event from the adapter
* @crq: crq to process
@@ -3273,6 +3527,7 @@ static 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,"
@@ -3363,12 +3618,28 @@ 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:
+ if (vhost->state == IBMVFC_HOST_OFFLINE)
+ break;
+ aw = kzalloc_obj(*aw, GFP_ATOMIC);
+ if (aw) {
+ if (!scsi_host_get(vhost->host)) {
+ kfree(aw);
+ break;
+ }
+ 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;
}
}
-
/**
* ibmvfc_handle_crq - Handles and frees received events in the CRQ
* @crq: Command/Response queue
@@ -6900,9 +7171,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 = alloc_ordered_workqueue("%s-fpin-workq-%u", 0,
+ IBMVFC_NAME, shost->host_no);
+ if (!vhost->fpin_workq)
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);
@@ -6910,7 +7187,7 @@ static int ibmvfc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
dev_err(dev, "Couldn't create kernel thread: %ld\n",
PTR_ERR(vhost->work_thread));
rc = PTR_ERR(vhost->work_thread);
- goto free_host_mem;
+ goto free_workq;
}
if ((rc = ibmvfc_init_crq(vhost))) {
@@ -6946,7 +7223,8 @@ static int ibmvfc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
ibmvfc_release_crq_queue(vhost);
kill_kthread:
kthread_stop(vhost->work_thread);
-free_host_mem:
+free_workq:
+ destroy_workqueue(vhost->fpin_workq);
ibmvfc_free_mem(vhost);
free_scsi_host:
scsi_host_put(shost);
@@ -6988,6 +7266,7 @@ static void ibmvfc_remove(struct vio_dev *vdev)
ibmvfc_complete_purge(&purge);
ibmvfc_release_sub_crqs(vhost);
ibmvfc_release_crq_queue(vhost);
+ destroy_workqueue(vhost->fpin_workq);
ibmvfc_free_mem(vhost);
spin_lock(&ibmvfc_driver_lock);
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.h b/drivers/scsi/ibmvscsi/ibmvfc.h
index 421ce8bc3ea3..3fffd2db8ef5 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.h
+++ b/drivers/scsi/ibmvscsi/ibmvfc.h
@@ -750,8 +750,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;
@@ -781,6 +785,12 @@ struct ibmvfc_async_sub_crq {
} id;
} __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;
@@ -1022,6 +1032,7 @@ struct ibmvfc_host {
wait_queue_head_t work_wait_q;
struct nvme_fc_local_port *nvme_local_port;
struct completion nvme_delete_done;
+ struct workqueue_struct *fpin_workq;
};
struct ibmvfc_event *__ibmvfc_get_event(struct ibmvfc_queue *queue, int reserved);
@@ -1090,6 +1101,7 @@ static inline struct ibmvfc_host *ibmvfc_channels_to_vhost(struct ibmvfc_channel
#if IS_ENABLED(CONFIG_KUNIT)
#include <kunit/visibility.h>
+VISIBLE_IF_KUNIT void ibmvfc_process_async_work(struct work_struct *work);
VISIBLE_IF_KUNIT void ibmvfc_deregister_channel(struct ibmvfc_host *vhost,
struct ibmvfc_channels *channels,
int index);
diff --git a/drivers/scsi/ibmvscsi/ibmvfc_kunit.c b/drivers/scsi/ibmvscsi/ibmvfc_kunit.c
index 24799aa2368a..8f261b8672b1 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc_kunit.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc_kunit.c
@@ -3,10 +3,182 @@
#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"
MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
+/**
+ * ibmvfc_run_async_work - queue and drain an FPIN work item via a private workqueue
+ * @wq: private workqueue to use
+ * @vhost: ibmvfc host struct
+ * @ae: async CRQ event to process
+ *
+ * Allocates an ibmvfc_async_work, takes a scsi_host reference (released by
+ * ibmvfc_process_async_work()), populates the work item, queues it on @wq
+ * (which the caller owns), and flushes @wq so the work completes before
+ * returning. Using a private workqueue gives ibmvfc_process_async_work() its
+ * own thread stack, avoiding kernel stack overflow when called from a deep
+ * KUnit context.
+ */
+static void ibmvfc_run_async_work(struct workqueue_struct *wq,
+ struct ibmvfc_host *vhost,
+ struct ibmvfc_async_crq *ae)
+{
+ struct ibmvfc_async_work *aw;
+
+ aw = kzalloc_obj(*aw);
+ if (WARN_ON(!aw))
+ return;
+ if (WARN_ON(!scsi_host_get(vhost->host))) {
+ kfree(aw);
+ return;
+ }
+ INIT_WORK(&aw->async_work_s, ibmvfc_process_async_work);
+ aw->vhost = vhost;
+ aw->crq = *ae;
+ queue_work(wq, &aw->async_work_s);
+ flush_workqueue(wq);
+}
+
+/**
+ * ibmvfc_async_fpin_test - unit test for IBMVFC_AE_FPIN handling via
+ * ibmvfc_process_async_work
+ * @test: pointer to kunit structure
+ *
+ * Tests
+ * - statistics updates on valid FPIN status values
+ * - statistics unchanged on invalid FPIN status value
+ *
+ * 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];
+ struct ibmvfc_async_crq ae;
+ enum ibmvfc_ae_fpin_status fs;
+ struct workqueue_struct *wq;
+ struct fc_host_attrs *fc_host;
+ struct ibmvfc_target *tgt;
+ struct ibmvfc_host *vhost;
+ struct fc_rport *rport;
+ unsigned long flags;
+
+ vhost = ibmvfc_get_first_vhost();
+ if (!vhost)
+ kunit_skip(test, "No ibmvfc devices available");
+
+ wq = alloc_ordered_workqueue("ibmvfc-kunit-fpin", 0);
+ if (!wq) {
+ scsi_host_put(vhost->host);
+ kunit_skip(test, "Failed to allocate workqueue");
+ }
+
+ spin_lock_irqsave(&vhost->host->host_lock, flags);
+ if (vhost->scsi_scrqs.num_targets < 1) {
+ spin_unlock_irqrestore(&vhost->host->host_lock, flags);
+ destroy_workqueue(wq);
+ scsi_host_put(vhost->host);
+ kunit_skip(test, "No targets");
+ }
+ tgt = NULL;
+ rport = NULL;
+ list_for_each_entry(tgt, &vhost->scsi_scrqs.targets, queue) {
+ if (tgt->rport &&
+ tgt->rport->port_state == FC_PORTSTATE_ONLINE &&
+ (tgt->rport->roles & (FC_PORT_ROLE_FCP_TARGET | FC_PORT_ROLE_NVME_TARGET))) {
+ rport = tgt->rport;
+ get_device(&rport->dev);
+ kref_get(&tgt->kref);
+ break;
+ }
+ }
+ if (!rport) {
+ spin_unlock_irqrestore(&vhost->host->host_lock, flags);
+ destroy_workqueue(wq);
+ scsi_host_put(vhost->host);
+ kunit_skip(test, "No suitable online target rport found");
+ }
+ spin_unlock_irqrestore(&vhost->host->host_lock, flags);
+
+ 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(rport->fpin_stats.cn_device_specific);
+ pre[IBMVFC_AE_FPIN_PORT_CLEARED] = READ_ONCE(rport->fpin_stats.cn_clear);
+ pre[IBMVFC_AE_FPIN_PORT_DEGRADED] = READ_ONCE(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++) {
+ ae.valid = 0x80;
+ ae.link_state = IBMVFC_AE_LS_LINK_UP;
+ ae.fpin_status = fs;
+ ae.event = cpu_to_be64(IBMVFC_AE_FPIN);
+ ae.scsi_id = cpu_to_be64(tgt->scsi_id);
+ ae.wwpn = cpu_to_be64(tgt->wwpn);
+ ae.node_name = cpu_to_be64(tgt->ids.node_name);
+ ibmvfc_run_async_work(wq, vhost, &ae);
+ }
+
+ post[IBMVFC_AE_FPIN_LINK_CONGESTED] = READ_ONCE(fc_host->fpin_stats.cn_device_specific);
+ post[IBMVFC_AE_FPIN_PORT_CONGESTED] = READ_ONCE(rport->fpin_stats.cn_device_specific);
+ post[IBMVFC_AE_FPIN_PORT_CLEARED] = READ_ONCE(rport->fpin_stats.cn_clear);
+ post[IBMVFC_AE_FPIN_PORT_DEGRADED] = READ_ONCE(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);
+
+ /* bad path */
+ pre[IBMVFC_AE_FPIN_LINK_CONGESTED] = READ_ONCE(fc_host->fpin_stats.cn_device_specific);
+ pre[IBMVFC_AE_FPIN_PORT_CONGESTED] = READ_ONCE(rport->fpin_stats.cn_device_specific);
+ pre[IBMVFC_AE_FPIN_PORT_CLEARED] = READ_ONCE(rport->fpin_stats.cn_clear);
+ pre[IBMVFC_AE_FPIN_PORT_DEGRADED] = READ_ONCE(rport->fpin_stats.li_failure_unknown);
+ pre[IBMVFC_AE_FPIN_CONGESTION_CLEARED] = READ_ONCE(fc_host->fpin_stats.cn_clear);
+
+ ae.valid = 0x80;
+ ae.link_state = IBMVFC_AE_LS_LINK_UP;
+ ae.fpin_status = 0; /* bad value */
+ ae.event = cpu_to_be64(IBMVFC_AE_FPIN);
+ ae.scsi_id = cpu_to_be64(tgt->scsi_id);
+ ae.wwpn = cpu_to_be64(tgt->wwpn);
+ ae.node_name = cpu_to_be64(tgt->ids.node_name);
+ ibmvfc_run_async_work(wq, vhost, &ae);
+
+ post[IBMVFC_AE_FPIN_LINK_CONGESTED] = READ_ONCE(fc_host->fpin_stats.cn_device_specific);
+ post[IBMVFC_AE_FPIN_PORT_CONGESTED] = READ_ONCE(rport->fpin_stats.cn_device_specific);
+ post[IBMVFC_AE_FPIN_PORT_CLEARED] = READ_ONCE(rport->fpin_stats.cn_clear);
+ post[IBMVFC_AE_FPIN_PORT_DEGRADED] = READ_ONCE(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]);
+
+ destroy_workqueue(wq);
+ kref_put(&tgt->kref, ibmvfc_release_tgt);
+ put_device(&rport->dev);
+ scsi_host_put(vhost->host);
+}
+
/**
* ibmvfc_deregister_channel_guard_test - unit test for ibmvfc_deregister_channel()
* guard conditions
@@ -58,6 +230,17 @@ static struct kunit_suite ibmvfc_channel_test_suite = {
};
kunit_test_init_section_suite(ibmvfc_channel_test_suite);
+static struct kunit_case ibmvfc_fpin_test_cases[] = {
+ KUNIT_CASE(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.56.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v16 05/12] scsi: ibmvfc: add basic FPIN support
2026-10-26 16:12 ` [PATCH v16 05/12] scsi: ibmvfc: add basic FPIN support Dave Marquardt via B4 Relay
@ 2026-10-26 16:24 ` sashiko-bot
2026-10-26 22:11 ` Dave Marquardt
0 siblings, 1 reply; 21+ messages in thread
From: sashiko-bot @ 2026-10-26 16:24 UTC (permalink / raw)
To: Dave Marquardt; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: ibmvfc: double free in ibmvfc_probe() when ibmvfc_alloc_mem() fails
--
commit 1bc41de0e60f763f9d00e952aa43dcb15b71cefb
Author: Dave Marquardt <davemarq@linux.ibm.com>
scsi: ibmvfc: add basic FPIN support
This commit adds basic Fabric Performance Impact Notification (FPIN) support
to the ibmvfc driver by offloading IBMVFC_AE_FPIN async events to a
per-adapter ordered workqueue and passing converted descriptors to
fc_host_fpin_rcv(). It also adds KUnit tests for FPIN event handling.
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
> index b63b341efb074..1cd39d36edf5c 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
[ ... ]
> @@ -6900,9 +7171,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 = alloc_ordered_workqueue("%s-fpin-workq-%u", 0,
> + IBMVFC_NAME, shost->host_no);
> + if (!vhost->fpin_workq)
> 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);
>
> @@ -6910,7 +7187,7 @@ static int ibmvfc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
> dev_err(dev, "Couldn't create kernel thread: %ld\n",
> PTR_ERR(vhost->work_thread));
> rc = PTR_ERR(vhost->work_thread);
> - goto free_host_mem;
> + goto free_workq;
> }
>
> if ((rc = ibmvfc_init_crq(vhost))) {
> @@ -6946,7 +7223,8 @@ static int ibmvfc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
> ibmvfc_release_crq_queue(vhost);
> kill_kthread:
> kthread_stop(vhost->work_thread);
> -free_host_mem:
> +free_workq:
> + destroy_workqueue(vhost->fpin_workq);
> ibmvfc_free_mem(vhost);
[Severity: High]
Does jumping to free_workq when ibmvfc_alloc_mem(vhost) fails in
ibmvfc_probe() cause a double free in ibmvfc_free_mem(vhost)?
When an allocation fails inside ibmvfc_alloc_mem(), it already unwinds and
frees everything allocated up to that point before returning -ENOMEM, leaving
non-NULL dangling pointers in vhost:
drivers/scsi/ibmvscsi/ibmvfc-core.c:ibmvfc_alloc_mem() {
...
free_tgt_pool:
mempool_destroy(vhost->tgt_pool);
free_trace:
kfree(vhost->trace);
free_nvme_disc_buffer:
ibmvfc_free_disc_buf(dev, &vhost->nvme_scrqs);
free_scsi_disc_buffer:
ibmvfc_free_disc_buf(dev, &vhost->scsi_scrqs);
free_login_buffer:
dma_free_coherent(dev, sizeof(*vhost->login_buf),
vhost->login_buf, vhost->login_buf_dma);
free_sg_pool:
dma_pool_destroy(vhost->sg_pool);
unmap_async_crq:
ibmvfc_free_queue(vhost, async_q);
nomem:
LEAVE;
return -ENOMEM;
}
In ibmvfc_probe(), jumping to free_workq when ibmvfc_alloc_mem() fails
destroys vhost->fpin_workq and then falls through into ibmvfc_free_mem(vhost),
which frees all of those host resources a second time and calls
dma_free_coherent() on NULL pointers such as vhost->channel_setup_buf.
Should ibmvfc_alloc_mem(vhost) be called before alloc_ordered_workqueue()
(or use separate error labels for freeing host memory and destroying
vhost->fpin_workq) so that an ibmvfc_alloc_mem() failure skips
ibmvfc_free_mem()?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261026-ibmvfc-fpin-support-v16-0-a900aa657ed5@linux.ibm.com?part=5
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v16 05/12] scsi: ibmvfc: add basic FPIN support
2026-10-26 16:24 ` sashiko-bot
@ 2026-10-26 22:11 ` Dave Marquardt
0 siblings, 0 replies; 21+ messages in thread
From: Dave Marquardt @ 2026-10-26 22:11 UTC (permalink / raw)
To: sashiko-bot; +Cc: sashiko-reviews, linux-scsi
sashiko-bot@kernel.org writes:
>> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
>> index b63b341efb074..1cd39d36edf5c 100644
>> --- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
>> +++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
>
> [ ... ]
>
>> @@ -6900,9 +7171,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 = alloc_ordered_workqueue("%s-fpin-workq-%u", 0,
>> + IBMVFC_NAME, shost->host_no);
>> + if (!vhost->fpin_workq)
>> 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);
>>
>> @@ -6910,7 +7187,7 @@ static int ibmvfc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
>> dev_err(dev, "Couldn't create kernel thread: %ld\n",
>> PTR_ERR(vhost->work_thread));
>> rc = PTR_ERR(vhost->work_thread);
>> - goto free_host_mem;
>> + goto free_workq;
>> }
>>
>> if ((rc = ibmvfc_init_crq(vhost))) {
>> @@ -6946,7 +7223,8 @@ static int ibmvfc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
>> ibmvfc_release_crq_queue(vhost);
>> kill_kthread:
>> kthread_stop(vhost->work_thread);
>> -free_host_mem:
>> +free_workq:
>> + destroy_workqueue(vhost->fpin_workq);
>> ibmvfc_free_mem(vhost);
>
> [Severity: High]
> Does jumping to free_workq when ibmvfc_alloc_mem(vhost) fails in
> ibmvfc_probe() cause a double free in ibmvfc_free_mem(vhost)?
>
> When an allocation fails inside ibmvfc_alloc_mem(), it already unwinds and
> frees everything allocated up to that point before returning -ENOMEM, leaving
> non-NULL dangling pointers in vhost:
>
> drivers/scsi/ibmvscsi/ibmvfc-core.c:ibmvfc_alloc_mem() {
> ...
> free_tgt_pool:
> mempool_destroy(vhost->tgt_pool);
> free_trace:
> kfree(vhost->trace);
> free_nvme_disc_buffer:
> ibmvfc_free_disc_buf(dev, &vhost->nvme_scrqs);
> free_scsi_disc_buffer:
> ibmvfc_free_disc_buf(dev, &vhost->scsi_scrqs);
> free_login_buffer:
> dma_free_coherent(dev, sizeof(*vhost->login_buf),
> vhost->login_buf, vhost->login_buf_dma);
> free_sg_pool:
> dma_pool_destroy(vhost->sg_pool);
> unmap_async_crq:
> ibmvfc_free_queue(vhost, async_q);
> nomem:
> LEAVE;
> return -ENOMEM;
> }
>
> In ibmvfc_probe(), jumping to free_workq when ibmvfc_alloc_mem() fails
> destroys vhost->fpin_workq and then falls through into ibmvfc_free_mem(vhost),
> which frees all of those host resources a second time and calls
> dma_free_coherent() on NULL pointers such as vhost->channel_setup_buf.
>
> Should ibmvfc_alloc_mem(vhost) be called before alloc_ordered_workqueue()
> (or use separate error labels for freeing host memory and destroying
> vhost->fpin_workq) so that an ibmvfc_alloc_mem() failure skips
> ibmvfc_free_mem()?
Yes, this is a valid issue. If ibmvfc_free_mem() zeroed out all the
pointers, this call to ibmvfc_free_mem() might be fine. v17 will
allocate the ordered workqueue after allocating memory
(ibmvfc_alloc_mem()) and use a separate label in order to skip
ibmvfc_free_mem() if ibmvfc_alloc_mem() fails.
-Dave
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v16 06/12] scsi: ibmvfc: add NOOP command support
2026-10-26 16:12 [PATCH v16 00/12] scsi: ibmvfc: make ibmvfc support FPIN messages Dave Marquardt via B4 Relay
` (4 preceding siblings ...)
2026-10-26 16:12 ` [PATCH v16 05/12] scsi: ibmvfc: add basic FPIN support Dave Marquardt via B4 Relay
@ 2026-10-26 16:12 ` Dave Marquardt via B4 Relay
2026-10-26 16:12 ` [PATCH v16 07/12] scsi: ibmvfc: add FPIN extended flag and async sub-CRQ queue handle Dave Marquardt via B4 Relay
` (5 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Dave Marquardt via B4 Relay @ 2026-10-26 16:12 UTC (permalink / raw)
To: Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy (CS GROUP), James E.J. Bottomley,
Martin K. Petersen, Tyrel Datwyler
Cc: linux-scsi, linuxppc-dev, linux-kernel, Dave Marquardt
From: Dave Marquardt <davemarq@linux.ibm.com>
Add support for handling IBMVFC_NOOP format CRQ messages from the
VIOS partner.
Advertise the client's ability to handle NOOP commands by setting the
IBMVFC_CAN_USE_NOOP_CMD capability in ibmvfc_set_login_info().
Handle and ignore IBMVFC_NOOP format messages in both ibmvfc_handle_crq()
and ibmvfc_handle_scrq(). In both handlers, log a rate-limited error if a
NOOP is received while in the IBMVFC_ACTIVE state without the partner
having advertised IBMVFC_SUPPORT_NOOP_CMD.
In ibmvfc_handle_scrq(), also fix a typo ("Got and invalid" -> "Got an
invalid"), add a missing return on invalid message types, and guard
against null event pointers before processing CRQ responses.
Change ibmvfc_handle_scrq() to take a struct ibmvfc_queue *scrq
parameter instead of struct ibmvfc_host *vhost, deriving vhost from
scrq->vhost internally. Use &scrq->evt_pool for event validation instead
of &evt->queue->evt_pool, avoiding a potential wild pointer dereference
if crq->ioba contains a non-zero garbage token before ibmvfc_valid_event()
has a chance to validate it.
Signed-off-by: Dave Marquardt <davemarq@linux.ibm.com>
Acked-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc-core.c | 36 ++++++++++++++++++++++++++++++------
1 file changed, 30 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
index 1cd39d36edf5..b52bf7863b2d 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
@@ -1580,7 +1580,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);
@@ -3706,6 +3708,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_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
@@ -4225,9 +4235,10 @@ static int ibmvfc_toggle_scrq_irq(struct ibmvfc_queue *scrq, int enable)
return rc;
}
-static void ibmvfc_handle_scrq(struct ibmvfc_crq *crq, struct ibmvfc_host *vhost,
+static void ibmvfc_handle_scrq(struct ibmvfc_crq *crq, struct ibmvfc_queue *scrq,
struct list_head *evt_doneq)
{
+ struct ibmvfc_host *vhost = scrq->vhost;
struct ibmvfc_event *evt = (struct ibmvfc_event *)be64_to_cpu(crq->ioba);
switch (crq->valid) {
@@ -4236,7 +4247,20 @@ 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_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;
+ }
+
+ if (unlikely(!evt)) {
+ dev_err(vhost->dev, "Received null event\n");
return;
}
@@ -4244,7 +4268,7 @@ static void ibmvfc_handle_scrq(struct ibmvfc_crq *crq, struct ibmvfc_host *vhost
* things we send. Make sure this response is to something we
* actually sent
*/
- if (unlikely(!ibmvfc_valid_event(&evt->queue->evt_pool, evt))) {
+ if (unlikely(!ibmvfc_valid_event(&scrq->evt_pool, evt))) {
dev_err(vhost->dev, "Returned correlation_token 0x%08llx is invalid!\n",
crq->ioba);
return;
@@ -4287,7 +4311,7 @@ static void ibmvfc_drain_sub_crq(struct ibmvfc_queue *scrq)
spin_lock_irqsave(scrq->q_lock, flags);
while (!done) {
while ((crq = ibmvfc_next_scrq(scrq)) != NULL) {
- ibmvfc_handle_scrq(crq, scrq->vhost, &evt_doneq);
+ ibmvfc_handle_scrq(crq, scrq, &evt_doneq);
crq->valid = 0;
wmb();
}
@@ -4295,7 +4319,7 @@ static void ibmvfc_drain_sub_crq(struct ibmvfc_queue *scrq)
ibmvfc_toggle_scrq_irq(scrq, 1);
if ((crq = ibmvfc_next_scrq(scrq)) != NULL) {
ibmvfc_toggle_scrq_irq(scrq, 0);
- ibmvfc_handle_scrq(crq, scrq->vhost, &evt_doneq);
+ ibmvfc_handle_scrq(crq, scrq, &evt_doneq);
crq->valid = 0;
wmb();
} else
--
2.56.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v16 07/12] scsi: ibmvfc: add FPIN extended flag and async sub-CRQ queue handle
2026-10-26 16:12 [PATCH v16 00/12] scsi: ibmvfc: make ibmvfc support FPIN messages Dave Marquardt via B4 Relay
` (5 preceding siblings ...)
2026-10-26 16:12 ` [PATCH v16 06/12] scsi: ibmvfc: add NOOP command support Dave Marquardt via B4 Relay
@ 2026-10-26 16:12 ` Dave Marquardt via B4 Relay
2026-10-26 16:12 ` [PATCH v16 08/12] scsi: ibmvfc: extend async event handlers for async sub-CRQ events Dave Marquardt via B4 Relay
` (4 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Dave Marquardt via B4 Relay @ 2026-10-26 16:12 UTC (permalink / raw)
To: Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy (CS GROUP), James E.J. Bottomley,
Martin K. Petersen, Tyrel Datwyler
Cc: linux-scsi, linuxppc-dev, linux-kernel, Dave Marquardt
From: Dave Marquardt <davemarq@linux.ibm.com>
Add IBMVFC_ASYNC_IS_FPIN_EXT flag bit (0x02) to ibmvfc_async_sub_crq.flags
to distinguish extended FPIN events from standard ones.
Add async_scrqs field of type ibmvfc_channels to ibmvfc_host to hold the
channel set for the asynchronous sub-CRQ, alongside the existing scsi_scrqs
and nvme_scrqs channels. This uses the same ibmvfc_channels infrastructure
as the SCSI and NVMe channel queues.
Add IBMVFC_ASYNC_SUB_CRQ_FMT to enum ibmvfc_msg_fmt and a corresponding
async_scrq pointer to union ibmvfc_msgs to support the new async sub-CRQ
message format.
Signed-off-by: Dave Marquardt <davemarq@linux.ibm.com>
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc.h | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.h b/drivers/scsi/ibmvscsi/ibmvfc.h
index 3fffd2db8ef5..2ecd60138d58 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.h
+++ b/drivers/scsi/ibmvscsi/ibmvfc.h
@@ -773,6 +773,7 @@ struct ibmvfc_async_sub_crq {
volatile u8 valid;
u8 flags;
#define IBMVFC_ASYNC_ID_IS_ASSOC_ID 0x01
+#define IBMVFC_ASYNC_IS_FPIN_EXT 0x02
u8 link_state;
u8 fpin_status;
__be16 event;
@@ -893,6 +894,7 @@ enum ibmvfc_msg_fmt {
IBMVFC_CRQ_FMT = 0,
IBMVFC_ASYNC_FMT,
IBMVFC_SUB_CRQ_FMT,
+ IBMVFC_ASYNC_SUB_CRQ_FMT,
};
union ibmvfc_msgs {
@@ -900,6 +902,7 @@ union ibmvfc_msgs {
struct ibmvfc_crq *crq;
struct ibmvfc_async_crq *async;
struct ibmvfc_sub_crq *scrq;
+ struct ibmvfc_async_sub_crq *async_scrq;
};
struct ibmvfc_queue {
@@ -992,6 +995,7 @@ struct ibmvfc_host {
struct ibmvfc_queue async_crq;
struct ibmvfc_channels scsi_scrqs;
struct ibmvfc_channels nvme_scrqs;
+ struct ibmvfc_channels async_scrqs;
struct ibmvfc_npiv_login login_info;
union ibmvfc_npiv_login_data *login_buf;
dma_addr_t login_buf_dma;
--
2.56.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v16 08/12] scsi: ibmvfc: extend async event handlers for async sub-CRQ events
2026-10-26 16:12 [PATCH v16 00/12] scsi: ibmvfc: make ibmvfc support FPIN messages Dave Marquardt via B4 Relay
` (6 preceding siblings ...)
2026-10-26 16:12 ` [PATCH v16 07/12] scsi: ibmvfc: add FPIN extended flag and async sub-CRQ queue handle Dave Marquardt via B4 Relay
@ 2026-10-26 16:12 ` Dave Marquardt via B4 Relay
2026-10-26 16:12 ` [PATCH v16 09/12] scsi: ibmvfc: add interrupt routine for asynchronous sub CRQ Dave Marquardt via B4 Relay
` (3 subsequent siblings)
11 siblings, 0 replies; 21+ messages in thread
From: Dave Marquardt via B4 Relay @ 2026-10-26 16:12 UTC (permalink / raw)
To: Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy (CS GROUP), James E.J. Bottomley,
Martin K. Petersen, Tyrel Datwyler
Cc: linux-scsi, linuxppc-dev, linux-kernel, 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.
Introduce struct ibmvfc_async_crq_event, a tagged union that wraps
either an ibmvfc_async_crq (main CRQ) or an ibmvfc_async_sub_crq
(async sub-CRQ), with an enum ibmvfc_async_crq_type discriminator.
Replace the ibmvfc_async_work bare union and is_subq bool with a
single event field of this type.
Modify ibmvfc_handle_async() to accept a struct ibmvfc_async_crq_event *
instead of a void * plus a bool flag. Update ibmvfc_process_async_work()
to dispatch based on event.type.
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 union member
based on event type.
Update KUnit tests to use ibmvfc_run_async_work() with a single reused
struct ibmvfc_async_crq_event rather than arrays at call sites.
Signed-off-by: Dave Marquardt <davemarq@linux.ibm.com>
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc-core.c | 161 +++++++++++++++++++++++++--------
drivers/scsi/ibmvscsi/ibmvfc.h | 15 ++-
drivers/scsi/ibmvscsi/ibmvfc_kunit.c | 171 +++++++++++++++++++++++++++++++----
3 files changed, 291 insertions(+), 56 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
index b52bf7863b2d..64981a5a39fb 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
@@ -3433,6 +3433,46 @@ 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
+ * @wwpn: attached port WWPN (host-endian)
+ *
+ * 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_sub_crq *ibmvfc_fpin, u64 wwpn)
+{
+ __be16 type;
+
+ switch (ibmvfc_fpin->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(ibmvfc_fpin->fpin_status, cpu_to_be64(wwpn),
+ type, cpu_to_be16(0),
+ cpu_to_be32(IBMVFC_FPIN_DEFAULT_EVENT_THRESHOLD),
+ cpu_to_be32(1));
+}
+
/**
* ibmvfc_find_target - Search for a target in a target list
* @target_list: list head of targets to search
@@ -3470,28 +3510,40 @@ static struct ibmvfc_target *ibmvfc_find_target(struct list_head *target_list,
*/
VISIBLE_IF_KUNIT void ibmvfc_process_async_work(struct work_struct *work)
{
+ struct ibmvfc_async_sub_crq *subq = NULL;
struct ibmvfc_async_work *aw;
- struct ibmvfc_async_crq *crq;
+ struct ibmvfc_async_crq *crq = NULL;
struct ibmvfc_target *tgt;
struct ibmvfc_host *vhost;
- struct fc_els_fpin *fpin;
+ struct fc_els_fpin *fpin = NULL;
unsigned long flags;
+ __be64 node_name;
+ __be64 scsi_id;
+ __be64 wwpn;
aw = container_of_const(work, struct ibmvfc_async_work, async_work_s);
vhost = aw->vhost;
- crq = &aw->crq;
+ if (aw->event.type == IBMVFC_ASYNC_CRQ_SUB) {
+ subq = &aw->event.subq;
+ scsi_id = 0;
+ wwpn = subq->wwpn;
+ node_name = (subq->flags & IBMVFC_ASYNC_ID_IS_ASSOC_ID) ? 0 : subq->id.node_name;
+ } else {
+ crq = &aw->event.async_crq;
+ scsi_id = crq->scsi_id;
+ wwpn = crq->wwpn;
+ node_name = crq->node_name;
+ }
- if (!crq->scsi_id && !crq->wwpn && !crq->node_name)
+ if (!scsi_id && !wwpn && !node_name)
goto free;
spin_lock_irqsave(&vhost->host->host_lock, flags);
- tgt = ibmvfc_find_target(&vhost->scsi_scrqs.targets, crq->scsi_id,
- crq->wwpn, crq->node_name);
+ tgt = ibmvfc_find_target(&vhost->scsi_scrqs.targets, scsi_id, wwpn, node_name);
+
if (!tgt) {
/* Target not found in scsi_scrqs, search nvme_scrqs */
- tgt = ibmvfc_find_target(&vhost->nvme_scrqs.targets,
- crq->scsi_id, crq->wwpn,
- crq->node_name);
+ tgt = ibmvfc_find_target(&vhost->nvme_scrqs.targets, scsi_id, wwpn, node_name);
}
if (tgt) {
@@ -3503,7 +3555,11 @@ VISIBLE_IF_KUNIT void ibmvfc_process_async_work(struct work_struct *work)
goto free;
}
- 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, tgt->wwpn);
+
if (fpin) {
fc_host_fpin_rcv(tgt->vhost->host,
sizeof(*fpin) + be32_to_cpu(fpin->desc_len),
@@ -3521,25 +3577,51 @@ EXPORT_SYMBOL_IF_KUNIT(ibmvfc_process_async_work);
/**
* ibmvfc_handle_async - Handle an async event from the adapter
- * @crq: crq to process
+ * @ae: tagged union wrapping either an ibmvfc_async_crq (main CRQ) or an
+ * ibmvfc_async_sub_crq (async sub-CRQ); the type field identifies which
* @vhost: ibmvfc host struct
*
**/
-static void ibmvfc_handle_async(struct ibmvfc_async_crq *crq,
+static void ibmvfc_handle_async(struct ibmvfc_async_crq_event *ae,
struct ibmvfc_host *vhost)
{
- 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_sub_crq *subq = NULL;
+ const struct ibmvfc_async_desc *desc;
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 (ae->type == IBMVFC_ASYNC_CRQ_SUB) {
+ subq = &ae->subq;
+ event = (u64)be16_to_cpu(subq->event);
+ link_state = subq->link_state;
+ scsi_id = 0;
+ wwpn = subq->wwpn;
+ node_name = subq->flags & IBMVFC_ASYNC_ID_IS_ASSOC_ID ? 0 : subq->id.node_name;
+ } else {
+ async_crq = &ae->async_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;
@@ -3578,33 +3660,33 @@ static void ibmvfc_handle_async(struct ibmvfc_async_crq *crq,
case IBMVFC_AE_ELS_PRLO:
case IBMVFC_AE_ELS_PLOGI:
list_for_each_entry(tgt, &vhost->scsi_scrqs.targets, queue) {
- if (!crq->scsi_id && !crq->wwpn && !crq->node_name)
+ 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);
}
}
list_for_each_entry(tgt, &vhost->nvme_scrqs.targets, queue) {
- if (!crq->scsi_id && !crq->wwpn && !crq->node_name)
+ 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);
}
@@ -3631,14 +3713,14 @@ static void ibmvfc_handle_async(struct ibmvfc_async_crq *crq,
}
INIT_WORK(&aw->async_work_s, ibmvfc_process_async_work);
aw->vhost = vhost;
- aw->crq = *crq;
+ aw->event = *ae;
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;
}
}
@@ -4165,6 +4247,9 @@ static void ibmvfc_tasklet(void *data)
{
struct ibmvfc_host *vhost = data;
struct vio_dev *vdev = to_vio_dev(vhost->dev);
+ struct ibmvfc_async_crq_event ae = {
+ .type = IBMVFC_ASYNC_CRQ_MAIN,
+ };
struct ibmvfc_crq *crq;
struct ibmvfc_async_crq *async;
struct ibmvfc_event *evt, *temp;
@@ -4177,7 +4262,8 @@ 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);
+ ae.async_crq = *async;
+ ibmvfc_handle_async(&ae, vhost);
async->valid = 0;
wmb();
}
@@ -4191,8 +4277,9 @@ static void ibmvfc_tasklet(void *data)
vio_enable_interrupts(vdev);
if ((async = ibmvfc_next_async_crq(vhost)) != NULL) {
+ ae.async_crq = *async;
vio_disable_interrupts(vdev);
- ibmvfc_handle_async(async, vhost);
+ ibmvfc_handle_async(&ae, vhost);
async->valid = 0;
wmb();
} else if ((crq = ibmvfc_next_crq(vhost)) != NULL) {
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.h b/drivers/scsi/ibmvscsi/ibmvfc.h
index 2ecd60138d58..86b600ec57c7 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.h
+++ b/drivers/scsi/ibmvscsi/ibmvfc.h
@@ -786,9 +786,22 @@ struct ibmvfc_async_sub_crq {
} id;
} __packed __aligned(8);
+enum ibmvfc_async_crq_type {
+ IBMVFC_ASYNC_CRQ_MAIN = 0,
+ IBMVFC_ASYNC_CRQ_SUB,
+};
+
+struct ibmvfc_async_crq_event {
+ enum ibmvfc_async_crq_type type;
+ union {
+ struct ibmvfc_async_crq async_crq;
+ struct ibmvfc_async_sub_crq subq;
+ };
+};
+
struct ibmvfc_async_work {
struct ibmvfc_host *vhost;
- struct ibmvfc_async_crq crq;
+ struct ibmvfc_async_crq_event event;
struct work_struct async_work_s;
};
diff --git a/drivers/scsi/ibmvscsi/ibmvfc_kunit.c b/drivers/scsi/ibmvscsi/ibmvfc_kunit.c
index 8f261b8672b1..b867cfe3138d 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc_kunit.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc_kunit.c
@@ -3,7 +3,6 @@
#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"
@@ -25,7 +24,7 @@ MODULE_IMPORT_NS("EXPORTED_FOR_KUNIT_TESTING");
*/
static void ibmvfc_run_async_work(struct workqueue_struct *wq,
struct ibmvfc_host *vhost,
- struct ibmvfc_async_crq *ae)
+ struct ibmvfc_async_crq_event *ae)
{
struct ibmvfc_async_work *aw;
@@ -38,7 +37,7 @@ static void ibmvfc_run_async_work(struct workqueue_struct *wq,
}
INIT_WORK(&aw->async_work_s, ibmvfc_process_async_work);
aw->vhost = vhost;
- aw->crq = *ae;
+ aw->event = *ae;
queue_work(wq, &aw->async_work_s);
flush_workqueue(wq);
}
@@ -58,7 +57,7 @@ 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];
- struct ibmvfc_async_crq ae;
+ struct ibmvfc_async_crq_event ae = { .type = IBMVFC_ASYNC_CRQ_MAIN };
enum ibmvfc_ae_fpin_status fs;
struct workqueue_struct *wq;
struct fc_host_attrs *fc_host;
@@ -113,13 +112,13 @@ static void ibmvfc_async_fpin_test(struct kunit *test)
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++) {
- ae.valid = 0x80;
- ae.link_state = IBMVFC_AE_LS_LINK_UP;
- ae.fpin_status = fs;
- ae.event = cpu_to_be64(IBMVFC_AE_FPIN);
- ae.scsi_id = cpu_to_be64(tgt->scsi_id);
- ae.wwpn = cpu_to_be64(tgt->wwpn);
- ae.node_name = cpu_to_be64(tgt->ids.node_name);
+ ae.async_crq.valid = 0x80;
+ ae.async_crq.link_state = IBMVFC_AE_LS_LINK_UP;
+ ae.async_crq.fpin_status = fs;
+ ae.async_crq.event = cpu_to_be64(IBMVFC_AE_FPIN);
+ ae.async_crq.scsi_id = cpu_to_be64(tgt->scsi_id);
+ ae.async_crq.wwpn = cpu_to_be64(tgt->wwpn);
+ ae.async_crq.node_name = cpu_to_be64(tgt->ids.node_name);
ibmvfc_run_async_work(wq, vhost, &ae);
}
@@ -147,13 +146,13 @@ static void ibmvfc_async_fpin_test(struct kunit *test)
pre[IBMVFC_AE_FPIN_PORT_DEGRADED] = READ_ONCE(rport->fpin_stats.li_failure_unknown);
pre[IBMVFC_AE_FPIN_CONGESTION_CLEARED] = READ_ONCE(fc_host->fpin_stats.cn_clear);
- ae.valid = 0x80;
- ae.link_state = IBMVFC_AE_LS_LINK_UP;
- ae.fpin_status = 0; /* bad value */
- ae.event = cpu_to_be64(IBMVFC_AE_FPIN);
- ae.scsi_id = cpu_to_be64(tgt->scsi_id);
- ae.wwpn = cpu_to_be64(tgt->wwpn);
- ae.node_name = cpu_to_be64(tgt->ids.node_name);
+ ae.async_crq.valid = 0x80;
+ ae.async_crq.link_state = IBMVFC_AE_LS_LINK_UP;
+ ae.async_crq.fpin_status = 0; /* bad value */
+ ae.async_crq.event = cpu_to_be64(IBMVFC_AE_FPIN);
+ ae.async_crq.scsi_id = cpu_to_be64(tgt->scsi_id);
+ ae.async_crq.wwpn = cpu_to_be64(tgt->wwpn);
+ ae.async_crq.node_name = cpu_to_be64(tgt->ids.node_name);
ibmvfc_run_async_work(wq, vhost, &ae);
post[IBMVFC_AE_FPIN_LINK_CONGESTED] = READ_ONCE(fc_host->fpin_stats.cn_device_specific);
@@ -230,8 +229,144 @@ static struct kunit_suite ibmvfc_channel_test_suite = {
};
kunit_test_init_section_suite(ibmvfc_channel_test_suite);
+
+/**
+ * ibmvfc_full_fpin_test - unit test for sub-CRQ FPIN handling via
+ * ibmvfc_process_async_work
+ * @test: pointer to kunit structure
+ *
+ * Tests
+ * - statistics updates on valid FPIN status values
+ * - statistics unchanged on invalid FPIN status value
+ *
+ * Return: void
+ */
+static void ibmvfc_full_fpin_test(struct kunit *test)
+{
+ u64 post[IBMVFC_AE_FPIN_CONGESTION_CLEARED + 1];
+ u64 pre[IBMVFC_AE_FPIN_CONGESTION_CLEARED + 1];
+ struct ibmvfc_async_crq_event ae = { .type = IBMVFC_ASYNC_CRQ_SUB };
+ enum ibmvfc_ae_fpin_status fs;
+ struct workqueue_struct *wq;
+ struct fc_host_attrs *fc_host;
+ struct ibmvfc_target *tgt;
+ struct ibmvfc_host *vhost;
+ struct fc_rport *rport;
+ unsigned long flags;
+
+ vhost = ibmvfc_get_first_vhost();
+ if (!vhost)
+ kunit_skip(test, "No ibmvfc devices available");
+
+ wq = alloc_ordered_workqueue("ibmvfc-kunit-fpin", 0);
+ if (!wq) {
+ scsi_host_put(vhost->host);
+ kunit_skip(test, "Failed to allocate workqueue");
+ }
+
+ spin_lock_irqsave(&vhost->host->host_lock, flags);
+ if (vhost->scsi_scrqs.num_targets < 1) {
+ spin_unlock_irqrestore(&vhost->host->host_lock, flags);
+ destroy_workqueue(wq);
+ scsi_host_put(vhost->host);
+ kunit_skip(test, "No targets");
+ }
+ tgt = NULL;
+ rport = NULL;
+ list_for_each_entry(tgt, &vhost->scsi_scrqs.targets, queue) {
+ if (tgt->rport &&
+ tgt->rport->port_state == FC_PORTSTATE_ONLINE &&
+ (tgt->rport->roles & (FC_PORT_ROLE_FCP_TARGET | FC_PORT_ROLE_NVME_TARGET))) {
+ rport = tgt->rport;
+ get_device(&rport->dev);
+ kref_get(&tgt->kref);
+ break;
+ }
+ }
+ if (!rport) {
+ spin_unlock_irqrestore(&vhost->host->host_lock, flags);
+ destroy_workqueue(wq);
+ scsi_host_put(vhost->host);
+ kunit_skip(test, "No suitable online target rport found");
+ }
+ spin_unlock_irqrestore(&vhost->host->host_lock, flags);
+
+ 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(rport->fpin_stats.cn_device_specific);
+ pre[IBMVFC_AE_FPIN_PORT_CLEARED] = READ_ONCE(rport->fpin_stats.cn_clear);
+ pre[IBMVFC_AE_FPIN_PORT_DEGRADED] = READ_ONCE(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++) {
+ ae.subq.valid = 0x80;
+ ae.subq.link_state = IBMVFC_AE_LS_LINK_UP;
+ ae.subq.fpin_status = fs;
+ ae.subq.event = cpu_to_be16(IBMVFC_AE_FPIN);
+ ae.subq.wwpn = cpu_to_be64(tgt->wwpn);
+ ae.subq.id.node_name = cpu_to_be64(tgt->ids.node_name);
+ ibmvfc_run_async_work(wq, vhost, &ae);
+ }
+
+ post[IBMVFC_AE_FPIN_LINK_CONGESTED] = READ_ONCE(fc_host->fpin_stats.cn_device_specific);
+ post[IBMVFC_AE_FPIN_PORT_CONGESTED] = READ_ONCE(rport->fpin_stats.cn_device_specific);
+ post[IBMVFC_AE_FPIN_PORT_CLEARED] = READ_ONCE(rport->fpin_stats.cn_clear);
+ post[IBMVFC_AE_FPIN_PORT_DEGRADED] = READ_ONCE(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);
+
+ /* bad path */
+ pre[IBMVFC_AE_FPIN_LINK_CONGESTED] = READ_ONCE(fc_host->fpin_stats.cn_device_specific);
+ pre[IBMVFC_AE_FPIN_PORT_CONGESTED] = READ_ONCE(rport->fpin_stats.cn_device_specific);
+ pre[IBMVFC_AE_FPIN_PORT_CLEARED] = READ_ONCE(rport->fpin_stats.cn_clear);
+ pre[IBMVFC_AE_FPIN_PORT_DEGRADED] = READ_ONCE(rport->fpin_stats.li_failure_unknown);
+ pre[IBMVFC_AE_FPIN_CONGESTION_CLEARED] = READ_ONCE(fc_host->fpin_stats.cn_clear);
+
+ ae.subq.valid = 0x80;
+ ae.subq.link_state = IBMVFC_AE_LS_LINK_UP;
+ ae.subq.fpin_status = 0; /* bad value */
+ ae.subq.event = cpu_to_be16(IBMVFC_AE_FPIN);
+ ae.subq.wwpn = cpu_to_be64(tgt->wwpn);
+ ae.subq.id.node_name = cpu_to_be64(tgt->ids.node_name);
+ ibmvfc_run_async_work(wq, vhost, &ae);
+
+ post[IBMVFC_AE_FPIN_LINK_CONGESTED] = READ_ONCE(fc_host->fpin_stats.cn_device_specific);
+ post[IBMVFC_AE_FPIN_PORT_CONGESTED] = READ_ONCE(rport->fpin_stats.cn_device_specific);
+ post[IBMVFC_AE_FPIN_PORT_CLEARED] = READ_ONCE(rport->fpin_stats.cn_clear);
+ post[IBMVFC_AE_FPIN_PORT_DEGRADED] = READ_ONCE(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]);
+
+ destroy_workqueue(wq);
+ put_device(&rport->dev);
+ kref_put(&tgt->kref, ibmvfc_release_tgt);
+ scsi_host_put(vhost->host);
+}
+
static struct kunit_case ibmvfc_fpin_test_cases[] = {
KUNIT_CASE(ibmvfc_async_fpin_test),
+ KUNIT_CASE(ibmvfc_full_fpin_test),
{},
};
--
2.56.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v16 09/12] scsi: ibmvfc: add interrupt routine for asynchronous sub CRQ
2026-10-26 16:12 [PATCH v16 00/12] scsi: ibmvfc: make ibmvfc support FPIN messages Dave Marquardt via B4 Relay
` (7 preceding siblings ...)
2026-10-26 16:12 ` [PATCH v16 08/12] scsi: ibmvfc: extend async event handlers for async sub-CRQ events Dave Marquardt via B4 Relay
@ 2026-10-26 16:12 ` Dave Marquardt via B4 Relay
2026-10-26 16:24 ` sashiko-bot
2026-10-26 16:12 ` [PATCH v16 10/12] scsi: ibmvfc: extend channel reg/dereg helpers for async sub-CRQ Dave Marquardt via B4 Relay
` (2 subsequent siblings)
11 siblings, 1 reply; 21+ messages in thread
From: Dave Marquardt via B4 Relay @ 2026-10-26 16:12 UTC (permalink / raw)
To: Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy (CS GROUP), James E.J. Bottomley,
Martin K. Petersen, Tyrel Datwyler
Cc: linux-scsi, linuxppc-dev, linux-kernel, Dave Marquardt
From: Dave Marquardt <davemarq@linux.ibm.com>
Add ibmvfc_interrupt_async_subq(), an IRQ handler dedicated to
asynchronous sub-CRQ events from the adapter. The handler disables
the sub-CRQ IRQ and then calls ibmvfc_drain_async_subq() to consume
all pending entries before re-enabling interrupts.
Add ibmvfc_next_async_scrq() to traverse the async sub-CRQ ring
directly via msgs.async_scrq[], returning a typed
struct ibmvfc_async_sub_crq * without the cast that would be needed
when going through ibmvfc_next_scrq().
ibmvfc_drain_async_subq() holds the host lock and per-queue q_lock
while processing. It loops over available entries via
ibmvfc_next_async_scrq(), wrapping each in a struct
ibmvfc_async_crq_event and dispatching to ibmvfc_handle_async(),
then clears the valid bit and issues a write barrier. After draining,
it re-enables the sub-CRQ IRQ and performs one final check for a
newly arrived entry to close the IRQ-enable race; if one is found it
is processed before exiting the loop.
Signed-off-by: Dave Marquardt <davemarq@linux.ibm.com>
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc-core.c | 66 +++++++++++++++++++++++++++++++++++++
1 file changed, 66 insertions(+)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
index 64981a5a39fb..2cb38757333a 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
@@ -4387,6 +4387,72 @@ static struct ibmvfc_crq *ibmvfc_next_scrq(struct ibmvfc_queue *scrq)
return crq;
}
+static struct ibmvfc_async_sub_crq *ibmvfc_next_async_scrq(struct ibmvfc_queue *scrq)
+{
+ struct ibmvfc_async_sub_crq *crq;
+
+ crq = &scrq->msgs.async_scrq[scrq->cur];
+ if (crq->valid & 0x80) {
+ if (++scrq->cur == scrq->size)
+ scrq->cur = 0;
+ rmb();
+ } else
+ crq = NULL;
+
+ return crq;
+}
+
+static void ibmvfc_drain_async_subq(struct ibmvfc_queue *scrq)
+{
+ struct ibmvfc_host *vhost = scrq->vhost;
+ struct ibmvfc_async_crq_event ae = {
+ .type = IBMVFC_ASYNC_CRQ_SUB,
+ };
+ struct ibmvfc_async_sub_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_async_scrq(scrq)) != NULL) {
+ ae.subq = *crq;
+ ibmvfc_handle_async(&ae, scrq->vhost);
+ crq->valid = 0;
+ wmb(); /* complete write */
+ }
+
+ ibmvfc_toggle_scrq_irq(scrq, 1);
+ crq = ibmvfc_next_async_scrq(scrq);
+ if (crq != NULL) {
+ ae.subq = *crq;
+ ibmvfc_toggle_scrq_irq(scrq, 0);
+ ibmvfc_handle_async(&ae, scrq->vhost);
+ crq->valid = 0;
+ wmb(); /* complete write */
+ } else
+ done = 1;
+ }
+ spin_unlock(scrq->q_lock);
+ spin_unlock_irqrestore(&vhost->host->host_lock, flags);
+}
+
+/**
+ * ibmvfc_interrupt_async_subq - Handle an async event from the adapter
+ * @irq: interrupt request
+ * @scrq_instance: async subq
+ *
+ **/
+static irqreturn_t __maybe_unused 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;
--
2.56.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v16 09/12] scsi: ibmvfc: add interrupt routine for asynchronous sub CRQ
2026-10-26 16:12 ` [PATCH v16 09/12] scsi: ibmvfc: add interrupt routine for asynchronous sub CRQ Dave Marquardt via B4 Relay
@ 2026-10-26 16:24 ` sashiko-bot
2026-10-27 14:08 ` Dave Marquardt
0 siblings, 1 reply; 21+ messages in thread
From: sashiko-bot @ 2026-10-26 16:24 UTC (permalink / raw)
To: Dave Marquardt; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: ibmvfc: same-class q_lock nesting in ibmvfc_drain_async_subq()
--
commit d3f6ce8f09b129387eea8b24d92aacbad8e6f279
Author: Dave Marquardt <davemarq@linux.ibm.com>
scsi: ibmvfc: add interrupt routine for asynchronous sub CRQ
This commit adds ibmvfc_interrupt_async_subq(), ibmvfc_drain_async_subq(),
and ibmvfc_next_async_scrq() to handle and drain asynchronous sub-CRQ events
from the adapter.
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
> index 64981a5a39fbf..2cb38757333a4 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
> @@ -4387,6 +4387,72 @@ static struct ibmvfc_crq *ibmvfc_next_scrq(struct ibmvfc_queue *scrq)
> return crq;
> }
>
> +static struct ibmvfc_async_sub_crq *ibmvfc_next_async_scrq(struct ibmvfc_queue *scrq)
> +{
> + struct ibmvfc_async_sub_crq *crq;
> +
> + crq = &scrq->msgs.async_scrq[scrq->cur];
> + if (crq->valid & 0x80) {
> + if (++scrq->cur == scrq->size)
> + scrq->cur = 0;
> + rmb();
> + } else
> + crq = NULL;
> +
> + return crq;
> +}
> +
> +static void ibmvfc_drain_async_subq(struct ibmvfc_queue *scrq)
> +{
> + struct ibmvfc_host *vhost = scrq->vhost;
> + struct ibmvfc_async_crq_event ae = {
> + .type = IBMVFC_ASYNC_CRQ_SUB,
> + };
> + struct ibmvfc_async_sub_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_async_scrq(scrq)) != NULL) {
> + ae.subq = *crq;
> + ibmvfc_handle_async(&ae, scrq->vhost);
[Severity: High]
Can calling ibmvfc_handle_async() while holding scrq->q_lock trigger a
lockdep recursive locking warning?
Every struct ibmvfc_queue (including async_scrqs, scsi_scrqs, and nvme_scrqs)
initializes queue->_lock at the same call site in ibmvfc_alloc_queue():
spin_lock_init(&queue->_lock);
queue->q_lock = &queue->_lock;
This places scrq->q_lock, scsi_q[i].q_lock, and nvme_q[i].q_lock into the
same lockdep class.
When ibmvfc_drain_async_subq() processes a reset-triggering async event
(such as IBMVFC_AE_RESUME, IBMVFC_AE_LINK_UP, IBMVFC_AE_SCN_FABRIC, or
IBMVFC_AE_SCN_DOMAIN) while vhost->action is IBMVFC_HOST_ACTION_LOGO_WAIT
(for example, if a second async event arrives while NPIV logout is in flight
with vhost->using_channels set) or vhost->state is IBMVFC_HOST_OFFLINE, the
call chain reaches ibmvfc_purge_requests():
ibmvfc_drain_async_subq()
spin_lock(scrq->q_lock)
ibmvfc_handle_async()
__ibmvfc_reset_host()
ibmvfc_hard_reset_host()
ibmvfc_purge_requests()
In ibmvfc_purge_requests(), the loops over active SCSI and NVMe sub-queues
acquire scsi_q[i].q_lock and nvme_q[i].q_lock at subclass 0 while
scrq->q_lock of the same lockdep class is still held:
for (i = 0; i < shwqs; i++) {
spin_lock_irqsave(scsi_q[i].q_lock, flags);
spin_lock(&scsi_q[i].l_lock);
...
}
for (i = 0; i < nhwqs; i++) {
spin_lock_irqsave(nvme_q[i].q_lock, flags);
spin_lock(&nvme_q[i].l_lock);
...
}
Should async sub-queue locks use a separate lockdep class or subclass, or
should ibmvfc_handle_async() be called without holding scrq->q_lock?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261026-ibmvfc-fpin-support-v16-0-a900aa657ed5@linux.ibm.com?part=9
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v16 09/12] scsi: ibmvfc: add interrupt routine for asynchronous sub CRQ
2026-10-26 16:24 ` sashiko-bot
@ 2026-10-27 14:08 ` Dave Marquardt
0 siblings, 0 replies; 21+ messages in thread
From: Dave Marquardt @ 2026-10-27 14:08 UTC (permalink / raw)
To: sashiko-bot; +Cc: sashiko-reviews, linux-scsi
sashiko-bot@kernel.org writes:
>> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
>> index 64981a5a39fbf..2cb38757333a4 100644
>> --- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
>> +++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
>> @@ -4387,6 +4387,72 @@ static struct ibmvfc_crq *ibmvfc_next_scrq(struct ibmvfc_queue *scrq)
>> return crq;
>> }
>>
>> +static struct ibmvfc_async_sub_crq *ibmvfc_next_async_scrq(struct ibmvfc_queue *scrq)
>> +{
>> + struct ibmvfc_async_sub_crq *crq;
>> +
>> + crq = &scrq->msgs.async_scrq[scrq->cur];
>> + if (crq->valid & 0x80) {
>> + if (++scrq->cur == scrq->size)
>> + scrq->cur = 0;
>> + rmb();
>> + } else
>> + crq = NULL;
>> +
>> + return crq;
>> +}
>> +
>> +static void ibmvfc_drain_async_subq(struct ibmvfc_queue *scrq)
>> +{
>> + struct ibmvfc_host *vhost = scrq->vhost;
>> + struct ibmvfc_async_crq_event ae = {
>> + .type = IBMVFC_ASYNC_CRQ_SUB,
>> + };
>> + struct ibmvfc_async_sub_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_async_scrq(scrq)) != NULL) {
>> + ae.subq = *crq;
>> + ibmvfc_handle_async(&ae, scrq->vhost);
>
> [Severity: High]
> Can calling ibmvfc_handle_async() while holding scrq->q_lock trigger a
> lockdep recursive locking warning?
>
> Every struct ibmvfc_queue (including async_scrqs, scsi_scrqs, and nvme_scrqs)
> initializes queue->_lock at the same call site in ibmvfc_alloc_queue():
>
> spin_lock_init(&queue->_lock);
> queue->q_lock = &queue->_lock;
>
> This places scrq->q_lock, scsi_q[i].q_lock, and nvme_q[i].q_lock into the
> same lockdep class.
>
> When ibmvfc_drain_async_subq() processes a reset-triggering async event
> (such as IBMVFC_AE_RESUME, IBMVFC_AE_LINK_UP, IBMVFC_AE_SCN_FABRIC, or
> IBMVFC_AE_SCN_DOMAIN) while vhost->action is IBMVFC_HOST_ACTION_LOGO_WAIT
> (for example, if a second async event arrives while NPIV logout is in flight
> with vhost->using_channels set) or vhost->state is IBMVFC_HOST_OFFLINE, the
> call chain reaches ibmvfc_purge_requests():
>
> ibmvfc_drain_async_subq()
> spin_lock(scrq->q_lock)
> ibmvfc_handle_async()
> __ibmvfc_reset_host()
> ibmvfc_hard_reset_host()
> ibmvfc_purge_requests()
>
> In ibmvfc_purge_requests(), the loops over active SCSI and NVMe sub-queues
> acquire scsi_q[i].q_lock and nvme_q[i].q_lock at subclass 0 while
> scrq->q_lock of the same lockdep class is still held:
>
> for (i = 0; i < shwqs; i++) {
> spin_lock_irqsave(scsi_q[i].q_lock, flags);
> spin_lock(&scsi_q[i].l_lock);
> ...
> }
>
> for (i = 0; i < nhwqs; i++) {
> spin_lock_irqsave(nvme_q[i].q_lock, flags);
> spin_lock(&nvme_q[i].l_lock);
> ...
> }
>
> Should async sub-queue locks use a separate lockdep class or subclass, or
> should ibmvfc_handle_async() be called without holding scrq->q_lock?
I'm rewriting ibmvfc_drain_async_subq() to create a list of events to
handle, similar to the way ibmvfc_drain_sub_crq() does it, then unlock
the queue lock, and then call ibmvfc_handle_async() for each event. This
resolves the lockdep issue.
-Dave
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v16 10/12] scsi: ibmvfc: extend channel reg/dereg helpers for async sub-CRQ
2026-10-26 16:12 [PATCH v16 00/12] scsi: ibmvfc: make ibmvfc support FPIN messages Dave Marquardt via B4 Relay
` (8 preceding siblings ...)
2026-10-26 16:12 ` [PATCH v16 09/12] scsi: ibmvfc: add interrupt routine for asynchronous sub CRQ Dave Marquardt via B4 Relay
@ 2026-10-26 16:12 ` Dave Marquardt via B4 Relay
2026-10-26 16:12 ` [PATCH v16 11/12] scsi: ibmvfc: register and use asynchronous sub CRQ for events Dave Marquardt via B4 Relay
2026-10-26 16:12 ` [PATCH v16 12/12] scsi: ibmvfc: handle extended FPIN events Dave Marquardt via B4 Relay
11 siblings, 0 replies; 21+ messages in thread
From: Dave Marquardt via B4 Relay @ 2026-10-26 16:12 UTC (permalink / raw)
To: Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy (CS GROUP), James E.J. Bottomley,
Martin K. Petersen, Tyrel Datwyler
Cc: linux-scsi, linuxppc-dev, linux-kernel, Dave Marquardt
From: Dave Marquardt <davemarq@linux.ibm.com>
ibmvfc_register_channel() and ibmvfc_deregister_channel() previously only
handled SCSI and NVMe sub-CRQ channels identified by the channel protocol
(IBMVFC_PROTO_SCSI, IBMVFC_PROTO_NVME). The async sub-CRQ had no
registration path through these helpers, requiring separate handling.
Add a new IBMVFC_PROTO_ASYNC protocol value to enum ibmvfc_protocol.
Extend both functions to handle IBMVFC_PROTO_ASYNC channels: the IRQ is
named "ibmvfc-<addr>-async<index>" and the handler is set to
ibmvfc_interrupt_async_subq rather than the per-protocol
ibmvfc_interrupt_mq handler.
Remove the __maybe_unused annotation from ibmvfc_interrupt_async_subq
now that it is referenced in ibmvfc_register_channel().
Error messages in both paths are updated to include the channel protocol
to distinguish async sub-CRQ failures from SCSI/NVMe sub-CRQ failures.
Kernel-doc headers are added to both functions.
Signed-off-by: Dave Marquardt <davemarq@linux.ibm.com>
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc-core.c | 30 +++++++++++++++++++++++++-----
drivers/scsi/ibmvscsi/ibmvfc.h | 1 +
2 files changed, 26 insertions(+), 5 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
index 2cb38757333a..af11726b76be 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
@@ -4443,7 +4443,7 @@ static void ibmvfc_drain_async_subq(struct ibmvfc_queue *scrq)
* @scrq_instance: async subq
*
**/
-static irqreturn_t __maybe_unused ibmvfc_interrupt_async_subq(int irq, void *scrq_instance)
+static irqreturn_t ibmvfc_interrupt_async_subq(int irq, void *scrq_instance)
{
struct ibmvfc_queue *scrq = (struct ibmvfc_queue *)scrq_instance;
@@ -6833,6 +6833,19 @@ static int ibmvfc_init_crq(struct ibmvfc_host *vhost)
return retrc;
}
+/**
+ * ibmvfc_register_channel - Register a sub-CRQ channel with the hypervisor
+ * @vhost: ibmvfc host struct
+ * @channels: ibmvfc channels struct containing the channel array and protocol
+ * @index: index into the channels array for the queue to register
+ *
+ * Register a sub-CRQ with the hypervisor via h_reg_sub_crq, map its hardware
+ * IRQ to a Linux IRQ, and bind an interrupt handler to it. The handler is
+ * selected based on the channel protocol.
+ *
+ * Return value:
+ * 0 on success / non-zero on failure
+ **/
static int ibmvfc_register_channel(struct ibmvfc_host *vhost,
struct ibmvfc_channels *channels,
int index)
@@ -6860,7 +6873,8 @@ 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);
+ dev_err(dev, "Error mapping protocol (%d) sub-crq[%d] irq\n",
+ channels->protocol, index);
goto irq_failed;
}
@@ -6875,23 +6889,29 @@ static int ibmvfc_register_channel(struct ibmvfc_host *vhost,
vdev->unit_address, index);
scrq->handler = ibmvfc_interrupt_mq;
break;
+ case IBMVFC_PROTO_ASYNC:
+ snprintf(scrq->name, sizeof(scrq->name), "ibmvfc-%x-async%d",
+ vdev->unit_address, index);
+ scrq->handler = ibmvfc_interrupt_async_subq;
+ break;
default:
dev_err(dev, "Unknown channel protocol (%d)\n",
channels->protocol);
goto irq_failed;
}
+ 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);
+ dev_err(dev, "Couldn't register protocol (%d) sub-crq[%d] irq\n",
+ channels->protocol, index);
irq_dispose_mapping(scrq->irq);
scrq->irq = 0;
goto irq_failed;
}
- scrq->hwq_id = index;
-
LEAVE;
return 0;
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.h b/drivers/scsi/ibmvscsi/ibmvfc.h
index 86b600ec57c7..f77966c85b0e 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.h
+++ b/drivers/scsi/ibmvscsi/ibmvfc.h
@@ -839,6 +839,7 @@ enum ibmvfc_target_action {
enum ibmvfc_protocol {
IBMVFC_PROTO_SCSI = 0,
IBMVFC_PROTO_NVME = 1,
+ IBMVFC_PROTO_ASYNC = 2,
};
struct ibmvfc_target {
--
2.56.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v16 11/12] scsi: ibmvfc: register and use asynchronous sub CRQ for events
2026-10-26 16:12 [PATCH v16 00/12] scsi: ibmvfc: make ibmvfc support FPIN messages Dave Marquardt via B4 Relay
` (9 preceding siblings ...)
2026-10-26 16:12 ` [PATCH v16 10/12] scsi: ibmvfc: extend channel reg/dereg helpers for async sub-CRQ Dave Marquardt via B4 Relay
@ 2026-10-26 16:12 ` Dave Marquardt via B4 Relay
2026-10-26 16:31 ` sashiko-bot
2026-10-26 16:12 ` [PATCH v16 12/12] scsi: ibmvfc: handle extended FPIN events Dave Marquardt via B4 Relay
11 siblings, 1 reply; 21+ messages in thread
From: Dave Marquardt via B4 Relay @ 2026-10-26 16:12 UTC (permalink / raw)
To: Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy (CS GROUP), James E.J. Bottomley,
Martin K. Petersen, Tyrel Datwyler
Cc: linux-scsi, linuxppc-dev, linux-kernel, Dave Marquardt
From: Dave Marquardt <davemarq@linux.ibm.com>
Wire the async sub-CRQ into the sub-CRQ lifecycle so it is allocated,
registered, and freed alongside the SCSI channel queues.
In ibmvfc_probe(), initialize vhost->async_scrqs with desired_queues=1,
max_queues=1, and protocol=IBMVFC_PROTO_ASYNC. Add
ibmvfc_release_channels() as a helper to free a channels array and its
queue buffers.
In ibmvfc_init_sub_crqs(), allocate the async sub-CRQ via
ibmvfc_alloc_channels() before allocating the SCSI channels. Change
ibmvfc_reg_sub_crqs() to return int (0 on success, -1 on failure) so
ibmvfc_init_sub_crqs() can detect registration failure per channel set
and unwind correctly: async registration failure releases async_scrqs
and disables multi-queue; SCSI registration failure releases scsi_scrqs,
deregisters and releases async_scrqs, and disables multi-queue. In
ibmvfc_reg_sub_crqs(), also clear vhost->mq_enabled on registration
failure to prevent a host state machine stall: without this, mq_enabled
remaining set causes ibmvfc_npiv_login_done() to enter the multi-queue
branch while do_enquiry is 0, bypassing both ibmvfc_channel_enquiry()
and the fallback that would wake up the work queue, leaving the host
indefinitely in IBMVFC_HOST_ACTION_INIT_WAIT.
Both mq_enabled and do_enquiry are bitfields packed into the same word
as other state flags such as using_channels. Writes to these fields
from ibmvfc_reg_sub_crqs() and ibmvfc_init_sub_crqs() are performed
outside host_lock, while concurrent writers like
ibmvfc_channel_setup_done() hold host_lock when modifying other fields
in the same word. To prevent lost updates from concurrent
read-modify-write operations on the packed bitfield word, take
host_lock around all failure-path writes to mq_enabled, do_enquiry, and
nvme_enabled in ibmvfc_reg_sub_crqs() and ibmvfc_init_sub_crqs().
Remove vhost->mq_enabled from the ibmvfc_dereg_sub_crqs() guard,
relying solely on !channels->scrqs. The previous guard caused all
deregistration calls after a registration failure to be silently
skipped: since ibmvfc_reg_sub_crqs() clears mq_enabled on failure, any
previously registered channel sets (e.g. async_scrqs registered before
scsi_scrqs fails) would be leaked — their hypervisor sub-CRQ
registrations and IRQ handlers left active while
ibmvfc_release_channels() subsequently freed the underlying memory.
ibmvfc_deregister_channel() already guards individually on scrq->cookie
and scrq->irq, so iterating unconditionally over channels with scrqs
allocated is safe.
In ibmvfc_alloc_channels(), select IBMVFC_ASYNC_SUB_CRQ_FMT for async
protocol channels instead of IBMVFC_SUB_CRQ_FMT, so the queue is sized
for ibmvfc_async_sub_crq entries. Zero active_queues on allocation
failure to ensure the field is always consistent with scrqs.
In ibmvfc_release_sub_crqs(), ibmvfc_reenable_crq_queue(), and
ibmvfc_reset_crq(), deregister and re-register the async sub-CRQ
alongside the SCSI channel queues.
In ibmvfc_channel_setup_done(), compute async->active_queues from the
VIOS response: set it to 1 only when async->scrqs is non-NULL and the
response contains a non-zero async_sub_crq_handle, otherwise 0. Store
the handle in vhost->async_scrqs.scrqs[0].vios_cookie via the
active_queues loop. Clamp scsi->active_queues and nvme->active_queues
to their respective max_queues values before using them as loop bounds,
preventing out-of-bounds access to scrqs[] if the hypervisor returns a
larger value than was allocated. In ibmvfc_channel_setup(), guard the
access to async->scrqs[0].cookie behind a NULL check to prevent a
dereference when mq_enabled is 0 and async->scrqs was never allocated.
In ibmvfc_set_login_info(), advertise IBMVFC_USE_ASYNC_SUBQ,
IBMVFC_CAN_HANDLE_FPIN, and IBMVFC_YES_SCSI capabilities whenever
multi-queue channels are enabled. IBMVFC_YES_SCSI was previously only
set for NVMe-enabled configurations; move it to the common multi-queue
path so it is always advertised when channels are in use.
Signed-off-by: Dave Marquardt <davemarq@linux.ibm.com>
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc-core.c | 155 ++++++++++++++++++++++++++++--------
1 file changed, 121 insertions(+), 34 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
index af11726b76be..4d4ebe5f6f12 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
@@ -191,7 +191,7 @@ static void ibmvfc_tgt_implicit_logout_and_del(struct ibmvfc_target *);
static void ibmvfc_tgt_move_login(struct ibmvfc_target *);
static void ibmvfc_dereg_sub_crqs(struct ibmvfc_host *, struct ibmvfc_channels *);
-static void ibmvfc_reg_sub_crqs(struct ibmvfc_host *, struct ibmvfc_channels *);
+static int ibmvfc_reg_sub_crqs(struct ibmvfc_host *, struct ibmvfc_channels *);
static const char *unknown_error = "unknown error";
@@ -966,6 +966,7 @@ static int ibmvfc_reenable_crq_queue(struct ibmvfc_host *vhost)
struct vio_dev *vdev = to_vio_dev(vhost->dev);
unsigned long flags;
+ ibmvfc_dereg_sub_crqs(vhost, &vhost->async_scrqs);
ibmvfc_dereg_sub_crqs(vhost, &vhost->scsi_scrqs);
ibmvfc_dereg_sub_crqs(vhost, &vhost->nvme_scrqs);
@@ -988,6 +989,7 @@ static int ibmvfc_reenable_crq_queue(struct ibmvfc_host *vhost)
spin_unlock(vhost->crq.q_lock);
spin_unlock_irqrestore(&vhost->host->host_lock, flags);
+ ibmvfc_reg_sub_crqs(vhost, &vhost->async_scrqs);
ibmvfc_reg_sub_crqs(vhost, &vhost->scsi_scrqs);
ibmvfc_reg_sub_crqs(vhost, &vhost->nvme_scrqs);
@@ -1008,6 +1010,7 @@ static int ibmvfc_reset_crq(struct ibmvfc_host *vhost)
struct vio_dev *vdev = to_vio_dev(vhost->dev);
struct ibmvfc_queue *crq = &vhost->crq;
+ ibmvfc_dereg_sub_crqs(vhost, &vhost->async_scrqs);
ibmvfc_dereg_sub_crqs(vhost, &vhost->scsi_scrqs);
ibmvfc_dereg_sub_crqs(vhost, &vhost->nvme_scrqs);
@@ -1044,6 +1047,7 @@ static int ibmvfc_reset_crq(struct ibmvfc_host *vhost)
spin_unlock(vhost->crq.q_lock);
spin_unlock_irqrestore(&vhost->host->host_lock, flags);
+ ibmvfc_reg_sub_crqs(vhost, &vhost->async_scrqs);
ibmvfc_reg_sub_crqs(vhost, &vhost->scsi_scrqs);
ibmvfc_reg_sub_crqs(vhost, &vhost->nvme_scrqs);
@@ -1586,9 +1590,11 @@ 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);
+ login_info->capabilities |= cpu_to_be64(IBMVFC_USE_ASYNC_SUBQ);
+ login_info->capabilities |= cpu_to_be64(IBMVFC_CAN_HANDLE_FPIN);
+ login_info->capabilities |= cpu_to_be64(IBMVFC_YES_SCSI);
if (vhost->nvme_enabled) {
login_info->capabilities |= cpu_to_be64(IBMVFC_YES_NVMEOF);
- login_info->capabilities |= cpu_to_be64(IBMVFC_YES_SCSI);
login_info->capabilities |= cpu_to_be64(IBMVFC_CAN_USE_WWPN_ALL);
}
}
@@ -5804,6 +5810,7 @@ static void ibmvfc_channel_setup_done(struct ibmvfc_event *evt)
{
struct ibmvfc_host *vhost = evt->vhost;
struct ibmvfc_channel_setup *setup = vhost->channel_setup_buf;
+ struct ibmvfc_channels *async = &vhost->async_scrqs;
struct ibmvfc_channels *scsi = &vhost->scsi_scrqs;
struct ibmvfc_channels *nvme = &vhost->nvme_scrqs;
u32 mad_status = be16_to_cpu(evt->xfer_iu->channel_setup.common.status);
@@ -5817,8 +5824,11 @@ static void ibmvfc_channel_setup_done(struct ibmvfc_event *evt)
ibmvfc_dbg(vhost, "Channel Setup succeeded\n");
flags = be32_to_cpu(setup->flags);
vhost->do_enquiry = 0;
- scsi->active_queues = be32_to_cpu(setup->num_scsi_subq_channels);
- nvme->active_queues = be32_to_cpu(setup->num_nvme_subq_channels);
+ scsi->active_queues = min(be32_to_cpu(setup->num_scsi_subq_channels),
+ scsi->max_queues);
+ nvme->active_queues = min(be32_to_cpu(setup->num_nvme_subq_channels),
+ nvme->max_queues);
+ async->active_queues = (async->scrqs && setup->async_sub_crq_handle) ? 1 : 0;
if (flags & IBMVFC_CHANNELS_CANCELED) {
ibmvfc_dbg(vhost, "Channels Canceled\n");
@@ -5834,6 +5844,8 @@ static void ibmvfc_channel_setup_done(struct ibmvfc_event *evt)
for (i = 0; i < nvme->active_queues; i++)
nvme->scrqs[i].vios_cookie =
be64_to_cpu(setup->channel_handles[scsi->active_queues + i]);
+ for (i = 0; i < async->active_queues; i++)
+ async->scrqs[i].vios_cookie = be64_to_cpu(setup->async_sub_crq_handle);
ibmvfc_dbg(vhost, "Using %u SCSI channels\n",
scsi->active_queues);
@@ -5868,6 +5880,7 @@ static void ibmvfc_channel_setup(struct ibmvfc_host *vhost)
struct ibmvfc_event *evt = ibmvfc_get_reserved_event(&vhost->crq);
struct ibmvfc_channels *scsi = &vhost->scsi_scrqs;
struct ibmvfc_channels *nvme = &vhost->nvme_scrqs;
+ struct ibmvfc_channels *async = &vhost->async_scrqs;
unsigned int scsi_channels =
min(scsi->desired_queues, vhost->max_vios_scsi_channels);
unsigned int nvme_channels =
@@ -5893,6 +5906,8 @@ static void ibmvfc_channel_setup(struct ibmvfc_host *vhost)
for (i = 0; i < nvme_channels; i++)
setup_buf->channel_handles[scsi_channels + i] =
cpu_to_be64(nvme->scrqs[i].cookie);
+ if (async->scrqs)
+ setup_buf->async_sub_crq_handle = cpu_to_be64(async->scrqs[0].cookie);
}
ibmvfc_init_event(evt, ibmvfc_channel_setup_done, IBMVFC_MAD_FORMAT);
@@ -6740,6 +6755,9 @@ static int ibmvfc_alloc_queue(struct ibmvfc_host *vhost,
queue->evt_depth = scsi_qdepth;
queue->reserved_depth = IBMVFC_NUM_INTERNAL_SUBQ_REQ;
break;
+ case IBMVFC_ASYNC_SUB_CRQ_FMT:
+ fmt_size = sizeof(*queue->msgs.async_scrq);
+ break;
default:
dev_warn(dev, "Unknown command/response queue message format: %d\n", fmt);
return -EINVAL;
@@ -6976,25 +6994,45 @@ VISIBLE_IF_KUNIT void ibmvfc_deregister_channel(struct ibmvfc_host *vhost,
}
EXPORT_SYMBOL_IF_KUNIT(ibmvfc_deregister_channel);
-static void ibmvfc_reg_sub_crqs(struct ibmvfc_host *vhost,
+/**
+ * ibmvfc_reg_sub_crqs - Register all queues in a channel set with the hypervisor
+ * @vhost: ibmvfc host struct
+ * @channels: channel set to register
+ *
+ * Iterates over channels->max_queues entries and calls ibmvfc_register_channel()
+ * for each. If any registration fails, previously registered queues in this
+ * channel set are deregistered, vhost->mq_enabled and vhost->do_enquiry are
+ * cleared under host_lock, and -1 is returned. Returns 0 immediately (without
+ * iterating) if mq_enabled is 0 or channels->scrqs is NULL.
+ *
+ * Return:
+ * * %0 - success, or multi-queue disabled / channels not allocated
+ * * %-1 - channel registration failed
+ */
+static int ibmvfc_reg_sub_crqs(struct ibmvfc_host *vhost,
struct ibmvfc_channels *channels)
{
+ unsigned long flags;
int i, j;
ENTER;
if (!vhost->mq_enabled || !channels->scrqs)
- return;
+ return 0;
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);
+ spin_lock_irqsave(&vhost->host->host_lock, flags);
vhost->do_enquiry = 0;
- return;
+ vhost->mq_enabled = 0;
+ spin_unlock_irqrestore(&vhost->host->host_lock, flags);
+ return -1;
}
}
LEAVE;
+ return 0;
}
static void ibmvfc_dereg_sub_crqs(struct ibmvfc_host *vhost,
@@ -7003,7 +7041,7 @@ static void ibmvfc_dereg_sub_crqs(struct ibmvfc_host *vhost,
int i;
ENTER;
- if (!vhost->mq_enabled || !channels->scrqs)
+ if (!channels->scrqs)
return;
for (i = 0; i < channels->max_queues; i++)
@@ -7016,16 +7054,21 @@ static int ibmvfc_alloc_channels(struct ibmvfc_host *vhost,
struct ibmvfc_channels *channels)
{
struct ibmvfc_queue *scrq;
+ enum ibmvfc_msg_fmt fmt;
int i, j;
int rc = 0;
channels->scrqs = kzalloc_objs(*channels->scrqs, channels->max_queues);
- if (!channels->scrqs)
+ if (!channels->scrqs) {
+ channels->active_queues = 0;
return -ENOMEM;
+ }
+ fmt = (channels->protocol != IBMVFC_PROTO_ASYNC) ? IBMVFC_SUB_CRQ_FMT :
+ IBMVFC_ASYNC_SUB_CRQ_FMT;
for (i = 0; i < channels->max_queues; i++) {
scrq = &channels->scrqs[i];
- rc = ibmvfc_alloc_queue(vhost, scrq, IBMVFC_SUB_CRQ_FMT);
+ rc = ibmvfc_alloc_queue(vhost, scrq, fmt);
if (rc) {
for (j = i; j > 0; j--) {
scrq = &channels->scrqs[j - 1];
@@ -7041,30 +7084,6 @@ static int ibmvfc_alloc_channels(struct ibmvfc_host *vhost,
return rc;
}
-static void ibmvfc_init_sub_crqs(struct ibmvfc_host *vhost)
-{
- ENTER;
- if (!vhost->mq_enabled)
- return;
-
- if (ibmvfc_alloc_channels(vhost, &vhost->scsi_scrqs)) {
- vhost->do_enquiry = 0;
- vhost->mq_enabled = 0;
- return;
- }
-
- ibmvfc_reg_sub_crqs(vhost, &vhost->scsi_scrqs);
-
- if (vhost->nvme_enabled) {
- if (ibmvfc_alloc_channels(vhost, &vhost->nvme_scrqs))
- vhost->nvme_enabled = 0;
- else
- ibmvfc_reg_sub_crqs(vhost, &vhost->nvme_scrqs);
- }
-
- LEAVE;
-}
-
static void ibmvfc_release_channels(struct ibmvfc_host *vhost,
struct ibmvfc_channels *channels)
{
@@ -7083,9 +7102,74 @@ static void ibmvfc_release_channels(struct ibmvfc_host *vhost,
}
}
+/**
+ * ibmvfc_init_sub_crqs - Allocate and register all sub-CRQ channel sets
+ * @vhost: ibmvfc host struct
+ *
+ * Allocates and registers the async, SCSI, and (if nvme_enabled) NVMe
+ * sub-CRQ channel sets in order. Returns immediately if mq_enabled is 0.
+ *
+ * On failure at any stage, previously registered and allocated channel sets
+ * are deregistered and freed in reverse order, and vhost->mq_enabled and
+ * vhost->do_enquiry are cleared under host_lock to prevent the host state
+ * machine from stalling in IBMVFC_HOST_ACTION_INIT_WAIT.
+ */
+static void ibmvfc_init_sub_crqs(struct ibmvfc_host *vhost)
+{
+ unsigned long flags;
+
+ ENTER;
+ if (!vhost->mq_enabled)
+ return;
+
+ /* register async_sub_crq channel */
+ if (ibmvfc_alloc_channels(vhost, &vhost->async_scrqs))
+ goto out;
+
+ if (ibmvfc_reg_sub_crqs(vhost, &vhost->async_scrqs))
+ goto release_async_sub_crq;
+
+ if (ibmvfc_alloc_channels(vhost, &vhost->scsi_scrqs))
+ goto deregister_async_sub_crq;
+
+ if (ibmvfc_reg_sub_crqs(vhost, &vhost->scsi_scrqs))
+ goto release_scsi_sub_crqs;
+
+ if (vhost->nvme_enabled) {
+ if (ibmvfc_alloc_channels(vhost, &vhost->nvme_scrqs)) {
+ spin_lock_irqsave(&vhost->host->host_lock, flags);
+ vhost->nvme_enabled = 0;
+ spin_unlock_irqrestore(&vhost->host->host_lock, flags);
+ } else if (ibmvfc_reg_sub_crqs(vhost, &vhost->nvme_scrqs)) {
+ ibmvfc_release_channels(vhost, &vhost->nvme_scrqs);
+ spin_lock_irqsave(&vhost->host->host_lock, flags);
+ vhost->nvme_enabled = 0;
+ spin_unlock_irqrestore(&vhost->host->host_lock, flags);
+ }
+ }
+
+ LEAVE;
+ return;
+
+release_scsi_sub_crqs:
+ ibmvfc_release_channels(vhost, &vhost->scsi_scrqs);
+deregister_async_sub_crq:
+ ibmvfc_dereg_sub_crqs(vhost, &vhost->async_scrqs);
+release_async_sub_crq:
+ ibmvfc_release_channels(vhost, &vhost->async_scrqs);
+out:
+ spin_lock_irqsave(&vhost->host->host_lock, flags);
+ vhost->do_enquiry = 0;
+ vhost->mq_enabled = 0;
+ spin_unlock_irqrestore(&vhost->host->host_lock, flags);
+}
+
static void ibmvfc_release_sub_crqs(struct ibmvfc_host *vhost)
{
ENTER;
+ ibmvfc_dereg_sub_crqs(vhost, &vhost->async_scrqs);
+ ibmvfc_release_channels(vhost, &vhost->async_scrqs);
+
if (!vhost->scsi_scrqs.scrqs)
return;
@@ -7351,6 +7435,9 @@ static int ibmvfc_probe(struct vio_dev *vdev, const struct vio_device_id *id)
vhost->task_set = 1;
vhost->mq_enabled = mq_enabled;
+ vhost->async_scrqs.desired_queues = 1;
+ vhost->async_scrqs.max_queues = 1;
+ vhost->async_scrqs.protocol = IBMVFC_PROTO_ASYNC;
vhost->scsi_scrqs.desired_queues = min(shost->nr_hw_queues, nr_scsi_channels);
vhost->scsi_scrqs.max_queues = shost->nr_hw_queues;
vhost->scsi_scrqs.protocol = IBMVFC_PROTO_SCSI;
--
2.56.0
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v16 11/12] scsi: ibmvfc: register and use asynchronous sub CRQ for events
2026-10-26 16:12 ` [PATCH v16 11/12] scsi: ibmvfc: register and use asynchronous sub CRQ for events Dave Marquardt via B4 Relay
@ 2026-10-26 16:31 ` sashiko-bot
2026-10-27 15:00 ` Dave Marquardt
0 siblings, 1 reply; 21+ messages in thread
From: sashiko-bot @ 2026-10-26 16:31 UTC (permalink / raw)
To: Dave Marquardt; +Cc: linux-scsi
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: ibmvfc: broken error unwind and MQ disable in ibmvfc_reg_sub_crqs()
--
commit 0ac96c5239b66a2d4b7e04178af6f76c367e358d
Author: Dave Marquardt <davemarq@linux.ibm.com>
scsi: ibmvfc: register and use asynchronous sub CRQ for events
This commit wires the asynchronous sub-CRQ into the ibmvfc sub-CRQ lifecycle
so that it is allocated, registered, and released alongside the SCSI and NVMe
channel queues. It also updates channel setup and login capability flags to
advertise async sub-CRQ support.
> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
> index af11726b76beb..4d4ebe5f6f123 100644
> --- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
> +++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
[ ... ]
> @@ -988,6 +989,7 @@ static int ibmvfc_reenable_crq_queue(struct ibmvfc_host *vhost)
> spin_unlock(vhost->crq.q_lock);
> spin_unlock_irqrestore(&vhost->host->host_lock, flags);
>
> + ibmvfc_reg_sub_crqs(vhost, &vhost->async_scrqs);
> ibmvfc_reg_sub_crqs(vhost, &vhost->scsi_scrqs);
> ibmvfc_reg_sub_crqs(vhost, &vhost->nvme_scrqs);
>
[ ... ]
> @@ -1044,6 +1047,7 @@ static int ibmvfc_reset_crq(struct ibmvfc_host *vhost)
> spin_unlock(vhost->crq.q_lock);
> spin_unlock_irqrestore(&vhost->host->host_lock, flags);
>
> + ibmvfc_reg_sub_crqs(vhost, &vhost->async_scrqs);
> ibmvfc_reg_sub_crqs(vhost, &vhost->scsi_scrqs);
> ibmvfc_reg_sub_crqs(vhost, &vhost->nvme_scrqs);
>
[ ... ]
> @@ -6976,25 +6994,45 @@ VISIBLE_IF_KUNIT void ibmvfc_deregister_channel(struct ibmvfc_host *vhost,
> }
> EXPORT_SYMBOL_IF_KUNIT(ibmvfc_deregister_channel);
>
> -static void ibmvfc_reg_sub_crqs(struct ibmvfc_host *vhost,
[ ... ]
> +static int ibmvfc_reg_sub_crqs(struct ibmvfc_host *vhost,
> struct ibmvfc_channels *channels)
> {
> + unsigned long flags;
> int i, j;
>
> ENTER;
> if (!vhost->mq_enabled || !channels->scrqs)
> - return;
> + return 0;
>
> 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);
> + spin_lock_irqsave(&vhost->host->host_lock, flags);
> vhost->do_enquiry = 0;
> - return;
> + vhost->mq_enabled = 0;
> + spin_unlock_irqrestore(&vhost->host->host_lock, flags);
> + return -1;
> }
> }
>
> LEAVE;
> + return 0;
> }
[ ... ]
> @@ -7083,9 +7102,74 @@ static void ibmvfc_release_channels(struct ibmvfc_host *vhost,
> }
> }
>
[ ... ]
> +static void ibmvfc_init_sub_crqs(struct ibmvfc_host *vhost)
> +{
> + unsigned long flags;
> +
> + ENTER;
> + if (!vhost->mq_enabled)
> + return;
> +
> + /* register async_sub_crq channel */
> + if (ibmvfc_alloc_channels(vhost, &vhost->async_scrqs))
> + goto out;
> +
> + if (ibmvfc_reg_sub_crqs(vhost, &vhost->async_scrqs))
> + goto release_async_sub_crq;
> +
> + if (ibmvfc_alloc_channels(vhost, &vhost->scsi_scrqs))
> + goto deregister_async_sub_crq;
> +
> + if (ibmvfc_reg_sub_crqs(vhost, &vhost->scsi_scrqs))
> + goto release_scsi_sub_crqs;
> +
> + if (vhost->nvme_enabled) {
> + if (ibmvfc_alloc_channels(vhost, &vhost->nvme_scrqs)) {
> + spin_lock_irqsave(&vhost->host->host_lock, flags);
> + vhost->nvme_enabled = 0;
> + spin_unlock_irqrestore(&vhost->host->host_lock, flags);
> + } else if (ibmvfc_reg_sub_crqs(vhost, &vhost->nvme_scrqs)) {
> + ibmvfc_release_channels(vhost, &vhost->nvme_scrqs);
> + spin_lock_irqsave(&vhost->host->host_lock, flags);
> + vhost->nvme_enabled = 0;
> + spin_unlock_irqrestore(&vhost->host->host_lock, flags);
> + }
> + }
[Severity: High]
Does clearing vhost->do_enquiry and vhost->mq_enabled unconditionally inside
ibmvfc_reg_sub_crqs() cause inconsistent state and leaked sub-CRQ
registrations when a later channel set fails to register?
In ibmvfc_init_sub_crqs(), vhost->async_scrqs and vhost->scsi_scrqs are
allocated and registered first. When vhost->nvme_enabled is set and
ibmvfc_reg_sub_crqs(vhost, &vhost->nvme_scrqs) fails, ibmvfc_init_sub_crqs()
appears to intend to only disable NVMe by releasing vhost->nvme_scrqs and
setting vhost->nvme_enabled = 0.
However, because ibmvfc_reg_sub_crqs() has already set vhost->do_enquiry = 0
and vhost->mq_enabled = 0, multi-queue and channel enquiry are disabled for
the entire host while vhost->async_scrqs and vhost->scsi_scrqs remain
allocated and registered with the hypervisor with active IRQ handlers.
Similarly, in ibmvfc_reenable_crq_queue() and ibmvfc_reset_crq():
ibmvfc_reg_sub_crqs(vhost, &vhost->async_scrqs);
ibmvfc_reg_sub_crqs(vhost, &vhost->scsi_scrqs);
ibmvfc_reg_sub_crqs(vhost, &vhost->nvme_scrqs);
If registering vhost->async_scrqs succeeds, and a subsequent call to
ibmvfc_reg_sub_crqs() for vhost->scsi_scrqs or vhost->nvme_scrqs fails,
vhost->mq_enabled and vhost->do_enquiry are cleared to 0, but the already
registered channel sets are not deregistered via ibmvfc_dereg_sub_crqs().
Should the caller handle clearing vhost->mq_enabled and vhost->do_enquiry (or
unwinding previously registered channel sets) instead of clearing those flags
unconditionally in ibmvfc_reg_sub_crqs()?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261026-ibmvfc-fpin-support-v16-0-a900aa657ed5@linux.ibm.com?part=11
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v16 11/12] scsi: ibmvfc: register and use asynchronous sub CRQ for events
2026-10-26 16:31 ` sashiko-bot
@ 2026-10-27 15:00 ` Dave Marquardt
0 siblings, 0 replies; 21+ messages in thread
From: Dave Marquardt @ 2026-10-27 15:00 UTC (permalink / raw)
To: sashiko-bot; +Cc: sashiko-reviews, linux-scsi
sashiko-bot@kernel.org writes:
>> diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
>> index af11726b76beb..4d4ebe5f6f123 100644
>> --- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
>> +++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
> [ ... ]
>> @@ -988,6 +989,7 @@ static int ibmvfc_reenable_crq_queue(struct ibmvfc_host *vhost)
>> spin_unlock(vhost->crq.q_lock);
>> spin_unlock_irqrestore(&vhost->host->host_lock, flags);
>>
>> + ibmvfc_reg_sub_crqs(vhost, &vhost->async_scrqs);
>> ibmvfc_reg_sub_crqs(vhost, &vhost->scsi_scrqs);
>> ibmvfc_reg_sub_crqs(vhost, &vhost->nvme_scrqs);
>>
> [ ... ]
>> @@ -1044,6 +1047,7 @@ static int ibmvfc_reset_crq(struct ibmvfc_host *vhost)
>> spin_unlock(vhost->crq.q_lock);
>> spin_unlock_irqrestore(&vhost->host->host_lock, flags);
>>
>> + ibmvfc_reg_sub_crqs(vhost, &vhost->async_scrqs);
>> ibmvfc_reg_sub_crqs(vhost, &vhost->scsi_scrqs);
>> ibmvfc_reg_sub_crqs(vhost, &vhost->nvme_scrqs);
>>
> [ ... ]
>> @@ -6976,25 +6994,45 @@ VISIBLE_IF_KUNIT void ibmvfc_deregister_channel(struct ibmvfc_host *vhost,
>> }
>> EXPORT_SYMBOL_IF_KUNIT(ibmvfc_deregister_channel);
>>
>> -static void ibmvfc_reg_sub_crqs(struct ibmvfc_host *vhost,
> [ ... ]
>> +static int ibmvfc_reg_sub_crqs(struct ibmvfc_host *vhost,
>> struct ibmvfc_channels *channels)
>> {
>> + unsigned long flags;
>> int i, j;
>>
>> ENTER;
>> if (!vhost->mq_enabled || !channels->scrqs)
>> - return;
>> + return 0;
>>
>> 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);
>> + spin_lock_irqsave(&vhost->host->host_lock, flags);
>> vhost->do_enquiry = 0;
>> - return;
>> + vhost->mq_enabled = 0;
>> + spin_unlock_irqrestore(&vhost->host->host_lock, flags);
>> + return -1;
>> }
>> }
>>
>> LEAVE;
>> + return 0;
>> }
> [ ... ]
>> @@ -7083,9 +7102,74 @@ static void ibmvfc_release_channels(struct ibmvfc_host *vhost,
>> }
>> }
>>
> [ ... ]
>> +static void ibmvfc_init_sub_crqs(struct ibmvfc_host *vhost)
>> +{
>> + unsigned long flags;
>> +
>> + ENTER;
>> + if (!vhost->mq_enabled)
>> + return;
>> +
>> + /* register async_sub_crq channel */
>> + if (ibmvfc_alloc_channels(vhost, &vhost->async_scrqs))
>> + goto out;
>> +
>> + if (ibmvfc_reg_sub_crqs(vhost, &vhost->async_scrqs))
>> + goto release_async_sub_crq;
>> +
>> + if (ibmvfc_alloc_channels(vhost, &vhost->scsi_scrqs))
>> + goto deregister_async_sub_crq;
>> +
>> + if (ibmvfc_reg_sub_crqs(vhost, &vhost->scsi_scrqs))
>> + goto release_scsi_sub_crqs;
>> +
>> + if (vhost->nvme_enabled) {
>> + if (ibmvfc_alloc_channels(vhost, &vhost->nvme_scrqs)) {
>> + spin_lock_irqsave(&vhost->host->host_lock, flags);
>> + vhost->nvme_enabled = 0;
>> + spin_unlock_irqrestore(&vhost->host->host_lock, flags);
>> + } else if (ibmvfc_reg_sub_crqs(vhost, &vhost->nvme_scrqs)) {
>> + ibmvfc_release_channels(vhost, &vhost->nvme_scrqs);
>> + spin_lock_irqsave(&vhost->host->host_lock, flags);
>> + vhost->nvme_enabled = 0;
>> + spin_unlock_irqrestore(&vhost->host->host_lock, flags);
>> + }
>> + }
>
> [Severity: High]
> Does clearing vhost->do_enquiry and vhost->mq_enabled unconditionally inside
> ibmvfc_reg_sub_crqs() cause inconsistent state and leaked sub-CRQ
> registrations when a later channel set fails to register?
>
> In ibmvfc_init_sub_crqs(), vhost->async_scrqs and vhost->scsi_scrqs are
> allocated and registered first. When vhost->nvme_enabled is set and
> ibmvfc_reg_sub_crqs(vhost, &vhost->nvme_scrqs) fails, ibmvfc_init_sub_crqs()
> appears to intend to only disable NVMe by releasing vhost->nvme_scrqs and
> setting vhost->nvme_enabled = 0.
>
> However, because ibmvfc_reg_sub_crqs() has already set vhost->do_enquiry = 0
> and vhost->mq_enabled = 0, multi-queue and channel enquiry are disabled for
> the entire host while vhost->async_scrqs and vhost->scsi_scrqs remain
> allocated and registered with the hypervisor with active IRQ handlers.
>
> Similarly, in ibmvfc_reenable_crq_queue() and ibmvfc_reset_crq():
>
> ibmvfc_reg_sub_crqs(vhost, &vhost->async_scrqs);
> ibmvfc_reg_sub_crqs(vhost, &vhost->scsi_scrqs);
> ibmvfc_reg_sub_crqs(vhost, &vhost->nvme_scrqs);
>
> If registering vhost->async_scrqs succeeds, and a subsequent call to
> ibmvfc_reg_sub_crqs() for vhost->scsi_scrqs or vhost->nvme_scrqs fails,
> vhost->mq_enabled and vhost->do_enquiry are cleared to 0, but the already
> registered channel sets are not deregistered via ibmvfc_dereg_sub_crqs().
>
> Should the caller handle clearing vhost->mq_enabled and vhost->do_enquiry (or
> unwinding previously registered channel sets) instead of clearing those flags
> unconditionally in ibmvfc_reg_sub_crqs()?
The callers will handle deregistering sub CRQs and clearing
vhost->mq_enabled and vhost->do_enquiry in the next revision.
-Dave
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v16 12/12] scsi: ibmvfc: handle extended FPIN events
2026-10-26 16:12 [PATCH v16 00/12] scsi: ibmvfc: make ibmvfc support FPIN messages Dave Marquardt via B4 Relay
` (10 preceding siblings ...)
2026-10-26 16:12 ` [PATCH v16 11/12] scsi: ibmvfc: register and use asynchronous sub CRQ for events Dave Marquardt via B4 Relay
@ 2026-10-26 16:12 ` Dave Marquardt via B4 Relay
11 siblings, 0 replies; 21+ messages in thread
From: Dave Marquardt via B4 Relay @ 2026-10-26 16:12 UTC (permalink / raw)
To: Madhavan Srinivasan, Michael Ellerman, Nicholas Piggin,
Christophe Leroy (CS GROUP), James E.J. Bottomley,
Martin K. Petersen, Tyrel Datwyler
Cc: linux-scsi, linuxppc-dev, linux-kernel, 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, event counts, and severity.
Add ibmvfc_ext_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() and ibmvfc_handle_async()
to zero node_name and dispatch to ibmvfc_ext_fpin_to_desc() when the
IBMVFC_ASYNC_IS_FPIN_EXT flag is set.
Extend ibmvfc_common_fpin_to_desc() with a severity parameter so that
extended FPIN events can propagate the severity value from the VIOS when
IBMVFC_FPIN_SEVERITY_VALID is set, rather than always defaulting to
FPIN_CONGN_SEVERITY_WARNING.
Set the IBMVFC_CAN_HANDLE_FPIN_EXT capability bit (0x800) during login
to inform the VIOS that the client can process extended FPIN messages.
Add the IBMVFC_CAN_CLS_CRQ_PRE_DMP capability bit (0x400) definition
for the intervening protocol-defined capability. Also add the
IBMVFC_SUPPORT_FPIN_EXT response capability bit definition used to gate
extended FPIN processing.
In ibmvfc_process_async_work(), use goto to bypass the generic "FPIN
event received, unable to process" error message when a more specific
diagnostic has already been emitted for invalid or unsupported extended
FPIN events.
Add KUnit tests to exercise extended FPIN event handling across all
FPIN status and event type combinations.
struct ibmvfc_async_subq_fpin uses volatile on its valid field,
consistent with the existing ibmvfc_crq, ibmvfc_async_crq, and
ibmvfc_async_sub_crq structs in ibmvfc.h, all of which mark their
hardware-owned ring-buffer valid field volatile to prevent the compiler
from caching or eliminating reads of memory written directly by the VIOS.
Signed-off-by: Tyrel Datwyler <tyreld@linux.ibm.com>
Signed-off-by: Dave Marquardt <davemarq@linux.ibm.com>
---
drivers/scsi/ibmvscsi/ibmvfc-core.c | 88 +++++++++++++++++++--
drivers/scsi/ibmvscsi/ibmvfc.h | 31 ++++++++
drivers/scsi/ibmvscsi/ibmvfc_kunit.c | 148 +++++++++++++++++++++++++++++++++++
3 files changed, 259 insertions(+), 8 deletions(-)
diff --git a/drivers/scsi/ibmvscsi/ibmvfc-core.c b/drivers/scsi/ibmvscsi/ibmvfc-core.c
index 4d4ebe5f6f12..c8b12b4cf6cf 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc-core.c
+++ b/drivers/scsi/ibmvscsi/ibmvfc-core.c
@@ -1593,6 +1593,7 @@ static void ibmvfc_set_login_info(struct ibmvfc_host *vhost)
login_info->capabilities |= cpu_to_be64(IBMVFC_USE_ASYNC_SUBQ);
login_info->capabilities |= cpu_to_be64(IBMVFC_CAN_HANDLE_FPIN);
login_info->capabilities |= cpu_to_be64(IBMVFC_YES_SCSI);
+ login_info->capabilities |= cpu_to_be64(IBMVFC_CAN_HANDLE_FPIN_EXT);
if (vhost->nvme_enabled) {
login_info->capabilities |= cpu_to_be64(IBMVFC_YES_NVMEOF);
login_info->capabilities |= cpu_to_be64(IBMVFC_CAN_USE_WWPN_ALL);
@@ -3324,6 +3325,11 @@ static size_t ibmvfc_fpin_size_helper(u8 fpin_status)
* IBMVFC_AE_FPIN_PORT_DEGRADED only
* @event_count: link integrity event count (big-endian); used for
* IBMVFC_AE_FPIN_PORT_DEGRADED only
+ * @severity: congestion severity value; stored in congestion
+ * descriptors (IBMVFC_AE_FPIN_LINK_CONGESTED,
+ * IBMVFC_AE_FPIN_CONGESTION_CLEARED,
+ * IBMVFC_AE_FPIN_PORT_CONGESTED,
+ * IBMVFC_AE_FPIN_PORT_CLEARED)
*
* Return:
* NULL - unable to allocate structure or unrecognised fpin_status
@@ -3331,7 +3337,7 @@ static size_t ibmvfc_fpin_size_helper(u8 fpin_status)
*/
static struct fc_els_fpin *
ibmvfc_common_fpin_to_desc(u8 fpin_status, __be64 wwpn, __be16 type, __be16 modifier,
- __be32 threshold, __be32 event_count)
+ __be32 threshold, __be32 event_count, __u8 severity)
{
struct fc_fn_peer_congn_desc *pdesc;
struct fc_fn_congn_desc *cdesc;
@@ -3359,7 +3365,7 @@ ibmvfc_common_fpin_to_desc(u8 fpin_status, __be64 wwpn, __be16 type, __be16 modi
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;
+ cdesc->severity = severity;
break;
case IBMVFC_AE_FPIN_PORT_CONGESTED:
case IBMVFC_AE_FPIN_PORT_CLEARED:
@@ -3436,7 +3442,7 @@ ibmvfc_basic_fpin_to_desc(struct ibmvfc_async_crq *crq, u64 wwpn)
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));
+ cpu_to_be32(1), FPIN_CONGN_SEVERITY_WARNING);
}
/**
@@ -3476,7 +3482,47 @@ ibmvfc_full_fpin_to_desc(struct ibmvfc_async_sub_crq *ibmvfc_fpin, u64 wwpn)
return ibmvfc_common_fpin_to_desc(ibmvfc_fpin->fpin_status, cpu_to_be64(wwpn),
type, cpu_to_be16(0),
cpu_to_be32(IBMVFC_FPIN_DEFAULT_EVENT_THRESHOLD),
- cpu_to_be32(1));
+ cpu_to_be32(1), FPIN_CONGN_SEVERITY_WARNING);
+}
+
+/**
+ * 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
+ * @wwpn: attached port WWPN (host-endian)
+ *
+ * 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, u64 wwpn)
+{
+ 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;
+ __u8 severity = FPIN_CONGN_SEVERITY_WARNING;
+
+ 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;
+ if (flags & IBMVFC_FPIN_SEVERITY_VALID)
+ severity = ibmvfc_fpin->fpin_data.event_data.severity;
+
+ return ibmvfc_common_fpin_to_desc(ibmvfc_fpin->fpin_status,
+ cpu_to_be64(wwpn), type,
+ modifier, threshold, count,
+ severity);
}
/**
@@ -3517,6 +3563,7 @@ static struct ibmvfc_target *ibmvfc_find_target(struct list_head *target_list,
VISIBLE_IF_KUNIT void ibmvfc_process_async_work(struct work_struct *work)
{
struct ibmvfc_async_sub_crq *subq = NULL;
+ struct ibmvfc_async_subq_fpin *sqfpin;
struct ibmvfc_async_work *aw;
struct ibmvfc_async_crq *crq = NULL;
struct ibmvfc_target *tgt;
@@ -3533,7 +3580,10 @@ VISIBLE_IF_KUNIT void ibmvfc_process_async_work(struct work_struct *work)
subq = &aw->event.subq;
scsi_id = 0;
wwpn = subq->wwpn;
- node_name = (subq->flags & IBMVFC_ASYNC_ID_IS_ASSOC_ID) ? 0 : subq->id.node_name;
+ if (subq->flags & (IBMVFC_ASYNC_IS_FPIN_EXT | IBMVFC_ASYNC_ID_IS_ASSOC_ID))
+ node_name = 0;
+ else
+ node_name = subq->id.node_name;
} else {
crq = &aw->event.async_crq;
scsi_id = crq->scsi_id;
@@ -3563,8 +3613,26 @@ VISIBLE_IF_KUNIT 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, tgt->wwpn);
+ else {
+ static_assert(sizeof(struct ibmvfc_async_subq_fpin) ==
+ sizeof(struct ibmvfc_async_sub_crq));
+ static_assert(offsetof(struct ibmvfc_async_subq_fpin, fpin_data) ==
+ offsetof(struct ibmvfc_async_sub_crq, nport_id));
+ sqfpin = (struct ibmvfc_async_subq_fpin *)subq;
+ if ((subq->flags & IBMVFC_ASYNC_IS_FPIN_EXT) == 0) {
+ fpin = ibmvfc_full_fpin_to_desc(subq, tgt->wwpn);
+ } else if (!(sqfpin->fpin_data.flags & IBMVFC_FPIN_EVENT_TYPE_VALID)) {
+ dev_err_ratelimited(vhost->dev,
+ "Invalid extended FPIN event received\n");
+ goto skip_generic_error;
+ } else if (!ibmvfc_check_caps(vhost, IBMVFC_SUPPORT_FPIN_EXT)) {
+ dev_err_ratelimited(vhost->dev,
+ "Unexpected extended FPIN event received\n");
+ goto skip_generic_error;
+ } else {
+ fpin = ibmvfc_ext_fpin_to_desc(sqfpin, tgt->wwpn);
+ }
+ }
if (fpin) {
fc_host_fpin_rcv(tgt->vhost->host,
@@ -3574,6 +3642,7 @@ VISIBLE_IF_KUNIT void ibmvfc_process_async_work(struct work_struct *work)
} else
dev_err_ratelimited(vhost->dev, "FPIN event received, unable to process\n");
+ skip_generic_error:
kref_put(&tgt->kref, ibmvfc_release_tgt);
free:
scsi_host_put(aw->vhost->host);
@@ -3608,7 +3677,10 @@ static void ibmvfc_handle_async(struct ibmvfc_async_crq_event *ae,
link_state = subq->link_state;
scsi_id = 0;
wwpn = subq->wwpn;
- node_name = subq->flags & IBMVFC_ASYNC_ID_IS_ASSOC_ID ? 0 : subq->id.node_name;
+ if (subq->flags & (IBMVFC_ASYNC_IS_FPIN_EXT | IBMVFC_ASYNC_ID_IS_ASSOC_ID))
+ node_name = 0;
+ else
+ node_name = subq->id.node_name;
} else {
async_crq = &ae->async_crq;
event = be64_to_cpu(async_crq->event);
diff --git a/drivers/scsi/ibmvscsi/ibmvfc.h b/drivers/scsi/ibmvscsi/ibmvfc.h
index f77966c85b0e..bce5183f08a4 100644
--- a/drivers/scsi/ibmvscsi/ibmvfc.h
+++ b/drivers/scsi/ibmvscsi/ibmvfc.h
@@ -210,6 +210,8 @@ struct ibmvfc_npiv_login {
#define IBMVFC_CAN_USE_WWPN_ALL 0x080
#define IBMVFC_USE_ASYNC_SUBQ 0x100
#define IBMVFC_CAN_USE_NOOP_CMD 0x200
+#define IBMVFC_CAN_CLS_CRQ_PRE_DMP 0x400
+#define IBMVFC_CAN_HANDLE_FPIN_EXT 0x800
__be64 node_name;
struct srp_direct_buf async;
u8 partition_name[IBMVFC_MAX_NAME];
@@ -261,6 +263,7 @@ struct ibmvfc_npiv_login_resp {
#define IBMVFC_SUPPORT_WWPN_ALL 0x0400
#define IBMVFC_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;
@@ -786,6 +789,34 @@ struct ibmvfc_async_sub_crq {
} 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;
+ __be64 wwpn;
+ struct ibmvfc_fpin_data fpin_data;
+} __packed __aligned(8);
+
enum ibmvfc_async_crq_type {
IBMVFC_ASYNC_CRQ_MAIN = 0,
IBMVFC_ASYNC_CRQ_SUB,
diff --git a/drivers/scsi/ibmvscsi/ibmvfc_kunit.c b/drivers/scsi/ibmvscsi/ibmvfc_kunit.c
index b867cfe3138d..3ba3b2e37a9e 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"
@@ -364,9 +365,156 @@ static void ibmvfc_full_fpin_test(struct kunit *test)
scsi_host_put(vhost->host);
}
+#define IBMVFC_TEST_FPIN_EXT_TARGET(fs, ev, stat) do { \
+ struct ibmvfc_async_crq_event ae = { .type = IBMVFC_ASYNC_CRQ_SUB }; \
+ memset(&crq, 0, sizeof(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)); \
+ ae.subq = *(struct ibmvfc_async_sub_crq *)&crq; \
+ pre = READ_ONCE(rport->fpin_stats.stat); \
+ ibmvfc_run_async_work(wq, vhost, &ae); \
+ post = READ_ONCE(rport->fpin_stats.stat); \
+ KUNIT_EXPECT_GT(test, post, pre); \
+} while (0)
+
+#define IBMVFC_TEST_FPIN_EXT_HOST(fs, ev, stat) do { \
+ struct ibmvfc_async_crq_event ae = { .type = IBMVFC_ASYNC_CRQ_SUB }; \
+ memset(&crq, 0, sizeof(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)); \
+ ae.subq = *(struct ibmvfc_async_sub_crq *)&crq; \
+ pre = READ_ONCE(fc_host->fpin_stats.stat); \
+ ibmvfc_run_async_work(wq, vhost, &ae); \
+ post = READ_ONCE(fc_host->fpin_stats.stat); \
+ KUNIT_EXPECT_GT(test, post, pre); \
+} while (0)
+
+/**
+ * ibmvfc_extended_fpin_test - unit test for extended FPIN events
+ * @test: pointer to kunit structure
+ *
+ * Return: void
+ */
+static void ibmvfc_extended_fpin_test(struct kunit *test)
+{
+ enum ibmvfc_ae_fpin_status fs;
+ struct ibmvfc_async_subq_fpin crq;
+ struct workqueue_struct *wq;
+ struct ibmvfc_target *tgt;
+ struct ibmvfc_host *vhost;
+ struct fc_host_attrs *fc_host;
+ struct fc_rport *rport;
+ unsigned long flags;
+ u64 pre, post;
+
+ vhost = ibmvfc_get_first_vhost();
+ if (!vhost)
+ kunit_skip(test, "No ibmvfc devices available");
+
+ if (!ibmvfc_check_caps(vhost, IBMVFC_SUPPORT_FPIN_EXT)) {
+ scsi_host_put(vhost->host);
+ kunit_skip(test, "Extended FPIN not supported");
+ }
+
+ wq = alloc_ordered_workqueue("ibmvfc-kunit-fpin", 0);
+ if (!wq) {
+ scsi_host_put(vhost->host);
+ kunit_skip(test, "Failed to allocate workqueue");
+ }
+
+ spin_lock_irqsave(&vhost->host->host_lock, flags);
+ if (vhost->scsi_scrqs.num_targets < 1) {
+ spin_unlock_irqrestore(&vhost->host->host_lock, flags);
+ destroy_workqueue(wq);
+ scsi_host_put(vhost->host);
+ kunit_skip(test, "No targets");
+ }
+ tgt = NULL;
+ rport = NULL;
+ list_for_each_entry(tgt, &vhost->scsi_scrqs.targets, queue) {
+ if (tgt->rport &&
+ tgt->rport->port_state == FC_PORTSTATE_ONLINE &&
+ (tgt->rport->roles & (FC_PORT_ROLE_FCP_TARGET | FC_PORT_ROLE_NVME_TARGET))) {
+ rport = tgt->rport;
+ get_device(&rport->dev);
+ kref_get(&tgt->kref);
+ break;
+ }
+ }
+ if (!rport) {
+ spin_unlock_irqrestore(&vhost->host->host_lock, flags);
+ destroy_workqueue(wq);
+ scsi_host_put(vhost->host);
+ kunit_skip(test, "No suitable online target rport found");
+ }
+ spin_unlock_irqrestore(&vhost->host->host_lock, flags);
+
+ fc_host = shost_to_fc_host(vhost->host);
+
+ for (fs = IBMVFC_AE_FPIN_LINK_CONGESTED; fs <= IBMVFC_AE_FPIN_CONGESTION_CLEARED; fs++) {
+ switch (fs) {
+ case IBMVFC_AE_FPIN_LINK_CONGESTED:
+ IBMVFC_TEST_FPIN_EXT_HOST(fs, FPIN_CONGN_CLEAR, cn_clear);
+ IBMVFC_TEST_FPIN_EXT_HOST(fs, FPIN_CONGN_LOST_CREDIT, cn_lost_credit);
+ IBMVFC_TEST_FPIN_EXT_HOST(fs, FPIN_CONGN_CREDIT_STALL, cn_credit_stall);
+ IBMVFC_TEST_FPIN_EXT_HOST(fs, FPIN_CONGN_OVERSUBSCRIPTION,
+ cn_oversubscription);
+ IBMVFC_TEST_FPIN_EXT_HOST(fs, FPIN_CONGN_DEVICE_SPEC, cn_device_specific);
+ break;
+ case IBMVFC_AE_FPIN_PORT_CONGESTED:
+ IBMVFC_TEST_FPIN_EXT_TARGET(fs, FPIN_CONGN_CLEAR, cn_clear);
+ IBMVFC_TEST_FPIN_EXT_TARGET(fs, FPIN_CONGN_LOST_CREDIT, cn_lost_credit);
+ IBMVFC_TEST_FPIN_EXT_TARGET(fs, FPIN_CONGN_CREDIT_STALL, cn_credit_stall);
+ IBMVFC_TEST_FPIN_EXT_TARGET(fs, FPIN_CONGN_OVERSUBSCRIPTION,
+ cn_oversubscription);
+ IBMVFC_TEST_FPIN_EXT_TARGET(fs, FPIN_CONGN_DEVICE_SPEC, cn_device_specific);
+ break;
+ case IBMVFC_AE_FPIN_PORT_CLEARED:
+ IBMVFC_TEST_FPIN_EXT_TARGET(fs, FPIN_CONGN_CLEAR, cn_clear);
+ break;
+ case IBMVFC_AE_FPIN_CONGESTION_CLEARED:
+ IBMVFC_TEST_FPIN_EXT_HOST(fs, FPIN_CONGN_CLEAR, cn_clear);
+ break;
+ case IBMVFC_AE_FPIN_PORT_DEGRADED:
+ IBMVFC_TEST_FPIN_EXT_TARGET(fs, FPIN_LI_UNKNOWN, li_failure_unknown);
+ IBMVFC_TEST_FPIN_EXT_TARGET(fs, FPIN_LI_LINK_FAILURE,
+ li_link_failure_count);
+ IBMVFC_TEST_FPIN_EXT_TARGET(fs, FPIN_LI_LOSS_OF_SYNC,
+ li_loss_of_sync_count);
+ IBMVFC_TEST_FPIN_EXT_TARGET(fs, FPIN_LI_LOSS_OF_SIG,
+ li_loss_of_signals_count);
+ IBMVFC_TEST_FPIN_EXT_TARGET(fs, FPIN_LI_PRIM_SEQ_ERR,
+ li_prim_seq_err_count);
+ IBMVFC_TEST_FPIN_EXT_TARGET(fs, FPIN_LI_INVALID_TX_WD,
+ li_invalid_tx_word_count);
+ IBMVFC_TEST_FPIN_EXT_TARGET(fs, FPIN_LI_INVALID_CRC, li_invalid_crc_count);
+ IBMVFC_TEST_FPIN_EXT_TARGET(fs, FPIN_LI_DEVICE_SPEC, li_device_specific);
+ break;
+ }
+ }
+
+ destroy_workqueue(wq);
+ put_device(&rport->dev);
+ kref_put(&tgt->kref, ibmvfc_release_tgt);
+ scsi_host_put(vhost->host);
+}
+
static struct kunit_case ibmvfc_fpin_test_cases[] = {
KUNIT_CASE(ibmvfc_async_fpin_test),
KUNIT_CASE(ibmvfc_full_fpin_test),
+ KUNIT_CASE(ibmvfc_extended_fpin_test),
{},
};
--
2.56.0
^ permalink raw reply related [flat|nested] 21+ messages in thread