[PATCH net-next v2.2 0/5] eth fbnic: Add fbnic self tests

[PATCH net-next v2.2 0/5] eth fbnic: Add fbnic self tests

[mike.marciniszyn]

From: "Mike Marciniszyn (Meta)" <mike.marciniszyn@gmail.com>

This series adds self tests to test the registers, the
msix interrupts, the tlv, and the firmware mailbox.

This series assumes that the
[PATCH net-next 0/2] Add debugfs hooks [1]
is present.

When the self tests are run the with ethtool -t:

        ethtool -t eth0
        The test result is PASS
        The test extra info:
        Register test (offline)  0
        MSI-X Interrupt test (offline)   0
        FW mailbox test (on/offline)     0

Mike Marciniszyn (Meta) (5):
  net: export netif_open for self_test usage
  eth fbnic: Add register self test
  eth fbnic: Add msix self test
  eth fbnic: TLV support for use by MBX self test
  eth fbnic: Add mailbox self test

Link: https://patch.msgid.link/20260127200644.11640-1-mike.marciniszyn@gmail.com/ [1]

v2 - add enums for test return codes 2/5, 3/5, snf 5/5
   - correct placement of fbnic_nic forward in 2/5, 4/5

 drivers/net/ethernet/meta/fbnic/fbnic.h       |   1 +
 drivers/net/ethernet/meta/fbnic/fbnic_csr.c   | 128 ++++++++
 drivers/net/ethernet/meta/fbnic/fbnic_csr.h   |  19 ++
 .../net/ethernet/meta/fbnic/fbnic_ethtool.c   |  93 ++++++
 drivers/net/ethernet/meta/fbnic/fbnic_fw.c    | 100 +++++++
 drivers/net/ethernet/meta/fbnic/fbnic_fw.h    |  27 ++
 drivers/net/ethernet/meta/fbnic/fbnic_irq.c   | 154 ++++++++++
 drivers/net/ethernet/meta/fbnic/fbnic_tlv.c   | 276 ++++++++++++++++++
 drivers/net/ethernet/meta/fbnic/fbnic_tlv.h   |  27 ++
 net/core/dev.c                                |   1 +
 10 files changed, 855 insertions(+)

--
2.43.0

[PATCH net-next v2.2 1/5] net: export netif_open for self_test usage

[mike.marciniszyn]

From: "Mike Marciniszyn (Meta)" <mike.marciniszyn@gmail.com>

dev_open() already is exported, but drivers which use the netdev
instance lock need to use netif_open() instead. netif_close() is
also already exported [1] so this completes the pairing.

This export is required for the following fbnic self tests to
avoid calling ndo_stop() and ndo_open() in favor of the
more appropriate netif_open() and netif_close() that notifies
any listeners that the interface went down to test and is now
coming back up.

Link: https://patch.msgid.link/20250309215851.2003708-1-sdf@fomichev.me [1]
Signed-off-by: Mike Marciniszyn (Meta) <mike.marciniszyn@gmail.com>
---
v2 - no changes

 net/core/dev.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/core/dev.c b/net/core/dev.c
index 43de5af0d6ec..d1d5694d1ff0 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1724,6 +1724,7 @@ int netif_open(struct net_device *dev, struct netlink_ext_ack *extack)

 	return ret;
 }
+EXPORT_SYMBOL(netif_open);

 static void __dev_close_many(struct list_head *head)
 {
--
2.43.0

[PATCH net-next v2.2 2/5] eth fbnic: Add register self test

[mike.marciniszyn]

From: "Mike Marciniszyn (Meta)" <mike.marciniszyn@gmail.com>

The register test will be used to verify hardware is behaving as expected.

The test itself will have us writing to registers that should have no
side effects due to us resetting after the test has been completed.

While the test is being run the interface should be offline.

This patch counts on the first patch of this series to export netif_open()
and also ensures that the half close calls netif_close() to
avoid deadlock.

Signed-off-by: Mike Marciniszyn (Meta) <mike.marciniszyn@gmail.com>
---
v2 - add enum for test return codes
   - place forward of struct fbnic_dev after include with a blank line

 drivers/net/ethernet/meta/fbnic/fbnic.h       |  18 +++
 drivers/net/ethernet/meta/fbnic/fbnic_csr.c   | 128 ++++++++++++++++++
 drivers/net/ethernet/meta/fbnic/fbnic_csr.h   |  19 +++
 .../net/ethernet/meta/fbnic/fbnic_ethtool.c   |  50 +++++++
 3 files changed, 197 insertions(+)

diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_csr.c b/drivers/net/ethernet/meta/fbnic/fbnic_csr.c
index d9c0dc1c2af9..dc62d623e37c 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_csr.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_csr.c
@@ -147,3 +147,131 @@ int fbnic_csr_regs_len(struct fbnic_dev *fbd)

 	return len;
 }
+
+/* CSR register test data
+ *
+ * The register test will be used to verify hardware is behaving as expected.
+ *
+ * The test itself will have us writing to registers that should have no
+ * side effects due to us resetting after the test has been completed.
+ * While the test is being run the interface should be offline.
+ */
+struct fbnic_csr_reg_test_data {
+	int	reg;
+	u16	reg_offset;
+	u8	array_len;
+	u32	read;
+	u32	write;
+};
+
+#define FBNIC_QUEUE_REG_TEST(_name, _read, _write) { \
+	.reg = FBNIC_QUEUE(0) + FBNIC_QUEUE_##_name, \
+	.reg_offset = FBNIC_QUEUE_STRIDE, \
+	.array_len = 64, \
+	.read = _read, \
+	.write = _write \
+}
+
+static const struct fbnic_csr_reg_test_data pattern_test[] = {
+	FBNIC_QUEUE_REG_TEST(TWQ0_CTL, FBNIC_QUEUE_TWQ_CTL_RESET,
+			     FBNIC_QUEUE_TWQ_CTL_RESET),
+	FBNIC_QUEUE_REG_TEST(TWQ0_PTRS, 0, ~0),
+	FBNIC_QUEUE_REG_TEST(TWQ0_SIZE, FBNIC_QUEUE_TWQ_SIZE_MASK, ~0),
+	FBNIC_QUEUE_REG_TEST(TWQ0_BAL, FBNIC_QUEUE_BAL_MASK, ~0),
+	FBNIC_QUEUE_REG_TEST(TWQ0_BAH, ~0, ~0),
+	FBNIC_QUEUE_REG_TEST(TWQ1_CTL, FBNIC_QUEUE_TWQ_CTL_RESET,
+			     FBNIC_QUEUE_TWQ_CTL_RESET),
+	FBNIC_QUEUE_REG_TEST(TWQ1_PTRS, 0, ~0),
+	FBNIC_QUEUE_REG_TEST(TWQ1_SIZE, FBNIC_QUEUE_TWQ_SIZE_MASK, ~0),
+	FBNIC_QUEUE_REG_TEST(TWQ1_BAL, FBNIC_QUEUE_BAL_MASK, ~0),
+	FBNIC_QUEUE_REG_TEST(TWQ1_BAH, ~0, ~0),
+	FBNIC_QUEUE_REG_TEST(TCQ_CTL, FBNIC_QUEUE_TCQ_CTL_RESET,
+			     FBNIC_QUEUE_TCQ_CTL_RESET),
+	FBNIC_QUEUE_REG_TEST(TCQ_PTRS, 0, ~0),
+	FBNIC_QUEUE_REG_TEST(TCQ_SIZE, FBNIC_QUEUE_TCQ_SIZE_MASK, ~0),
+	FBNIC_QUEUE_REG_TEST(TCQ_BAL, FBNIC_QUEUE_BAL_MASK, ~0),
+	FBNIC_QUEUE_REG_TEST(TCQ_BAH, ~0, ~0),
+	FBNIC_QUEUE_REG_TEST(RCQ_CTL, FBNIC_QUEUE_RCQ_CTL_RESET,
+			     FBNIC_QUEUE_RCQ_CTL_RESET),
+	FBNIC_QUEUE_REG_TEST(RCQ_PTRS, 0, ~0),
+	FBNIC_QUEUE_REG_TEST(RCQ_SIZE, FBNIC_QUEUE_RCQ_SIZE_MASK, ~0),
+	FBNIC_QUEUE_REG_TEST(RCQ_BAL, FBNIC_QUEUE_BAL_MASK, ~0),
+	FBNIC_QUEUE_REG_TEST(RCQ_BAH, ~0, ~0),
+	FBNIC_QUEUE_REG_TEST(BDQ_CTL, FBNIC_QUEUE_BDQ_CTL_RESET,
+			     FBNIC_QUEUE_BDQ_CTL_RESET),
+	FBNIC_QUEUE_REG_TEST(BDQ_HPQ_PTRS, 0, ~0),
+	FBNIC_QUEUE_REG_TEST(BDQ_HPQ_SIZE, FBNIC_QUEUE_BDQ_SIZE_MASK, ~0),
+	FBNIC_QUEUE_REG_TEST(BDQ_HPQ_BAL, FBNIC_QUEUE_BAL_MASK, ~0),
+	FBNIC_QUEUE_REG_TEST(BDQ_HPQ_BAH, ~0, ~0),
+	FBNIC_QUEUE_REG_TEST(BDQ_PPQ_PTRS, 0, ~0),
+	FBNIC_QUEUE_REG_TEST(BDQ_PPQ_SIZE, FBNIC_QUEUE_BDQ_SIZE_MASK, ~0),
+	FBNIC_QUEUE_REG_TEST(BDQ_PPQ_BAL, FBNIC_QUEUE_BAL_MASK, ~0),
+	FBNIC_QUEUE_REG_TEST(BDQ_PPQ_BAH, ~0, ~0),
+};
+
+static enum fbnic_reg_self_test_codes
+fbnic_csr_reg_pattern_test(struct fbnic_dev *fbd, int index,
+			   const struct fbnic_csr_reg_test_data *test_data)
+{
+	static const u32 pattern[] = { ~0, 0x5A5A5A5A, 0xA5A5A5A5, 0};
+	enum fbnic_reg_self_test_codes reg;
+	int i;
+
+	reg = test_data->reg + test_data->reg_offset * index;
+	for (i = 0; i < ARRAY_SIZE(pattern); i++) {
+		u32 val = pattern[i] & test_data->write;
+		u32 result;
+
+		wr32(fbd, reg, val);
+		result = rd32(fbd, reg);
+		val &= test_data->read;
+
+		if (result == val)
+			continue;
+
+		dev_err(fbd->dev,
+			"%s: reg 0x%06X failed, expected 0x%08X received 0x%08X\n",
+			__func__, reg, val, result);
+
+		/* Note that FBNIC_INTR_STATUS(0) could be tested and fail
+		 * and the result would not be reported since the register
+		 * offset is 0. However as that register isn't included in
+		 * the register test that isn't an issue.
+		 */
+		return reg;
+	}
+
+	return FBNIC_REG_TEST_SUCCESS;
+}
+
+/**
+ * fbnic_csr_regs_test() - Verify behavior of NIC registers
+ * @fbd: device to test
+ *
+ * This function is meant to test the bit values of various registers in
+ * the NIC device. Specifically this test will verify which bits are
+ * writable and which ones are not. It will write varying patterns of bits
+ * to the registers testing for sticky bits, or bits that are writable but
+ * should not be.
+ *
+ * Return: FBNIC_REG_TEST_SUCCESS on success, register number on failure
+ **/
+enum fbnic_reg_self_test_codes fbnic_csr_regs_test(struct fbnic_dev *fbd)
+{
+	const struct fbnic_csr_reg_test_data *test_data;
+
+	for (test_data = pattern_test;
+	     test_data < pattern_test + ARRAY_SIZE(pattern_test); test_data++) {
+		u32 i;
+
+		for (i = 0; i < test_data->array_len; i++) {
+			enum fbnic_reg_self_test_codes reg =
+				fbnic_csr_reg_pattern_test(fbd, i, test_data);
+
+			if (reg)
+				return reg;
+		}
+	}
+
+	return FBNIC_REG_TEST_SUCCESS;
+}
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_csr.h b/drivers/net/ethernet/meta/fbnic/fbnic_csr.h
index b717db879cd3..7cfb42067327 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_csr.h
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_csr.h
@@ -6,6 +6,8 @@

 #include <linux/bitops.h>

+struct fbnic_dev;
+
 #define CSR_BIT(nr)		(1u << (nr))
 #define CSR_GENMASK(h, l)	GENMASK(h, l)

@@ -1210,4 +1212,21 @@ enum{
 	FBNIC_CSR_VERSION_V1_0_ASIC = 1,
 };

+/**
+ * enum fbnic_reg_self_test_codes - return codes from self test routines
+ *
+ * This is the code that is returned from the register self test
+ * routines.
+ *
+ * The test either returns success or the register number
+ * that failed during the test.
+ *
+ * @FBNIC_REG_TEST_SUCCESS: no errors
+ */
+enum fbnic_reg_self_test_codes {
+	FBNIC_REG_TEST_SUCCESS = 0,
+};
+
+enum fbnic_reg_self_test_codes fbnic_csr_regs_test(struct fbnic_dev *fbd);
+
 #endif /* _FBNIC_CSR_H_ */
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
index 11745a2d8a44..2e882dbd408d 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
@@ -126,6 +126,16 @@ static const struct fbnic_stat fbnic_gstrings_xdp_stats[] = {
 #define FBNIC_STATS_LEN \
 	(FBNIC_HW_STATS_LEN + FBNIC_XDP_STATS_LEN * FBNIC_MAX_XDPQS)

+enum fbnic_self_test_results {
+	TEST_REG = 0,
+};
+
+static const char fbnic_gstrings_self_test[][ETH_GSTRING_LEN] = {
+	[TEST_REG]	= "Register test (offline)",
+};
+
+#define FBNIC_TEST_LEN ARRAY_SIZE(fbnic_gstrings_self_test)
+
 static void
 fbnic_get_drvinfo(struct net_device *netdev, struct ethtool_drvinfo *drvinfo)
 {
@@ -475,6 +485,10 @@ static void fbnic_get_strings(struct net_device *dev, u32 sset, u8 *data)
 		for (i = 0; i < FBNIC_MAX_XDPQS; i++)
 			fbnic_get_xdp_queue_strings(&data, i);
 		break;
+	case ETH_SS_TEST:
+		memcpy(data, fbnic_gstrings_self_test,
+		       sizeof(fbnic_gstrings_self_test));
+		break;
 	}
 }

@@ -566,6 +580,8 @@ static int fbnic_get_sset_count(struct net_device *dev, int sset)
 	switch (sset) {
 	case ETH_SS_STATS:
 		return FBNIC_STATS_LEN;
+	case ETH_SS_TEST:
+		return FBNIC_TEST_LEN;
 	default:
 		return -EOPNOTSUPP;
 	}
@@ -1475,6 +1491,39 @@ fbnic_remove_rxfh_context(struct net_device *netdev,
 	return 0;
 }

+static int fbnic_ethtool_regs_test(struct net_device *netdev, u64 *data)
+{
+	struct fbnic_net *fbn = netdev_priv(netdev);
+	struct fbnic_dev *fbd = fbn->fbd;
+
+	*data = fbnic_csr_regs_test(fbd);
+
+	return !!*data;
+}
+
+static void fbnic_self_test(struct net_device *netdev,
+			    struct ethtool_test *eth_test, u64 *data)
+{
+	bool if_running = netif_running(netdev);
+
+	if (!(eth_test->flags & ETH_TEST_FL_OFFLINE)) {
+		data[TEST_REG] = 0;
+		return;
+	}
+
+	if (if_running)
+		netif_close(netdev);
+
+	if (fbnic_ethtool_regs_test(netdev, &data[TEST_REG]))
+		eth_test->flags |= ETH_TEST_FL_FAILED;
+
+	if (if_running && netif_open(netdev, NULL)) {
+		netdev_err(netdev,
+			   "Failed to re-initialize hardware following test\n");
+		eth_test->flags |= ETH_TEST_FL_FAILED;
+	}
+}
+
 static void fbnic_get_channels(struct net_device *netdev,
 			       struct ethtool_channels *ch)
 {
@@ -1893,6 +1942,7 @@ static const struct ethtool_ops fbnic_ethtool_ops = {
 	.get_pause_stats		= fbnic_get_pause_stats,
 	.get_pauseparam			= fbnic_phylink_get_pauseparam,
 	.set_pauseparam			= fbnic_phylink_set_pauseparam,
+	.self_test			= fbnic_self_test,
 	.get_strings			= fbnic_get_strings,
 	.get_ethtool_stats		= fbnic_get_ethtool_stats,
 	.get_sset_count			= fbnic_get_sset_count,
--
2.43.0

[PATCH net-next v2.2 3/5] eth fbnic: Add msix self test

[mike.marciniszyn]

From: "Mike Marciniszyn (Meta)" <mike.marciniszyn@gmail.com>

This function is meant to test the global interrupt registers and the
PCIe IP MSI-X functionality. It essentially goes through and tests
various combinations of the set, clear, and mask bits in order to
verify the behavior is as we expect it to be from the driver.

Signed-off-by: Mike Marciniszyn (Meta) <mike.marciniszyn@gmail.com>
---
v2 - add enum for test return codes

 drivers/net/ethernet/meta/fbnic/fbnic.h       |  18 ++
 .../net/ethernet/meta/fbnic/fbnic_ethtool.c   |  28 ++++
 drivers/net/ethernet/meta/fbnic/fbnic_irq.c   | 154 ++++++++++++++++++
 3 files changed, 212 insertions(+)

diff --git a/drivers/net/ethernet/meta/fbnic/fbnic.h b/drivers/net/ethernet/meta/fbnic/fbnic.h
index 779a083b9215..092fa9ad93d0 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic.h
+++ b/drivers/net/ethernet/meta/fbnic/fbnic.h
@@ -194,6 +194,36 @@ void fbnic_synchronize_irq(struct fbnic_dev *fbd, int nr);
 int fbnic_request_irq(struct fbnic_dev *dev, int nr, irq_handler_t handler,
 		      unsigned long flags, const char *name, void *data);
 void fbnic_free_irq(struct fbnic_dev *dev, int nr, void *data);
+
+/**
+ * enum fbnic_msix_self_test_codes - return codes from self test routines
+ *
+ * These are the codes returned from the self test routines and
+ * stored in the test result array indexed by the specific
+ * test name.
+ *
+ * @FBNIC_TEST_MSIX_SUCCESS: no errors
+ * @FBNIC_TEST_MSIX_NOMEM: allocation failure
+ * @FBNIC_TEST_MSIX_IRQ_REQ_FAIL: IRQ request failure
+ * @FBNIC_TEST_MSIX_MASK: masking failed to prevent IRQ
+ * @FBNIC_TEST_MSIX_UNMASK: unmasking failure w/ sw status set
+ * @FBNIC_TEST_MSIX_IRQ_CLEAR: interrupt when clearing mask
+ * @FBNIC_TEST_MSIX_NO_INTERRUPT: no interrupt when not masked
+ * @FBNIC_TEST_MSIX_NO_CLEAR_OR_MASK: status not cleared, or mask not set
+ */
+enum fbnic_msix_self_test_codes {
+	FBNIC_TEST_MSIX_SUCCESS = 0,
+	FBNIC_TEST_MSIX_NOMEM = 5,
+	FBNIC_TEST_MSIX_IRQ_REQ_FAIL = 10,
+	FBNIC_TEST_MSIX_MASK = 20,
+	FBNIC_TEST_MSIX_UNMASK = 30,
+	FBNIC_TEST_MSIX_IRQ_CLEAR = 40,
+	FBNIC_TEST_MSIX_NO_INTERRUPT = 50,
+	FBNIC_TEST_MSIX_NO_CLEAR_OR_MASK = 60,
+};
+
+enum fbnic_msix_self_test_codes fbnic_msix_test(struct fbnic_dev *fbd);
+
 void fbnic_free_irqs(struct fbnic_dev *fbd);
 int fbnic_alloc_irqs(struct fbnic_dev *fbd);

diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
index 2e882dbd408d..b0c8d1b069e2 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
@@ -128,10 +128,12 @@ static const struct fbnic_stat fbnic_gstrings_xdp_stats[] = {

 enum fbnic_self_test_results {
 	TEST_REG = 0,
+	TEST_MSIX,
 };

 static const char fbnic_gstrings_self_test[][ETH_GSTRING_LEN] = {
 	[TEST_REG]	= "Register test (offline)",
+	[TEST_MSIX]	= "MSI-X Interrupt test (offline)",
 };

 #define FBNIC_TEST_LEN ARRAY_SIZE(fbnic_gstrings_self_test)
@@ -1501,6 +1503,28 @@ static int fbnic_ethtool_regs_test(struct net_device *netdev, u64 *data)
 	return !!*data;
 }

+/**
+ * fbnic_ethtool_msix_test - Verify behavior of NIC interrupts
+ * @netdev: netdev device to test
+ * @data: Pointer to results storage
+ *
+ * This function is meant to test the global interrupt registers and the
+ * PCIe IP MSI-X functionality. It essentially goes through and tests
+ * test various combinations of the set, clear, and mask bits in order to
+ * verify the behavior is as we expect it to be from the driver.
+ *
+ * Return: non-zero on failure.
+ **/
+static int fbnic_ethtool_msix_test(struct net_device *netdev, u64 *data)
+{
+	struct fbnic_net *fbn = netdev_priv(netdev);
+	struct fbnic_dev *fbd = fbn->fbd;
+
+	*data = fbnic_msix_test(fbd);
+
+	return !!*data;
+}
+
 static void fbnic_self_test(struct net_device *netdev,
 			    struct ethtool_test *eth_test, u64 *data)
 {
@@ -1508,6 +1532,7 @@ static void fbnic_self_test(struct net_device *netdev,

 	if (!(eth_test->flags & ETH_TEST_FL_OFFLINE)) {
 		data[TEST_REG] = 0;
+		data[TEST_MSIX] = 0;
 		return;
 	}

@@ -1517,6 +1542,9 @@ static void fbnic_self_test(struct net_device *netdev,
 	if (fbnic_ethtool_regs_test(netdev, &data[TEST_REG]))
 		eth_test->flags |= ETH_TEST_FL_FAILED;

+	if (fbnic_ethtool_msix_test(netdev, &data[TEST_MSIX]))
+		eth_test->flags |= ETH_TEST_FL_FAILED;
+
 	if (if_running && netif_open(netdev, NULL)) {
 		netdev_err(netdev,
 			   "Failed to re-initialize hardware following test\n");
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_irq.c b/drivers/net/ethernet/meta/fbnic/fbnic_irq.c
index 02e8b0b257fe..2099ee89fb51 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_irq.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_irq.c
@@ -238,6 +238,160 @@ void fbnic_free_irq(struct fbnic_dev *fbd, int nr, void *data)
 	free_irq(irq, data);
 }

+struct fbnic_msix_test_data {
+	struct fbnic_dev *fbd;
+	unsigned long test_msix_status[BITS_TO_LONGS(FBNIC_MAX_MSIX_VECS)];
+	int irq_vector[FBNIC_MAX_MSIX_VECS];
+};
+
+static irqreturn_t fbnic_irq_test(int irq, void *data)
+{
+	struct fbnic_msix_test_data *test_data = data;
+	struct fbnic_dev *fbd = test_data->fbd;
+	int i;
+
+	for (i = fbd->num_irqs; i--;) {
+		if (test_data->irq_vector[i] == irq) {
+			set_bit(i, test_data->test_msix_status);
+			break;
+		}
+	}
+
+	return IRQ_HANDLED;
+}
+
+/**
+ * fbnic_msix_test - Verify behavior of NIC interrupts
+ * @fbd: device to test
+ *
+ * This function is meant to test the global interrupt registers and the
+ * PCIe IP MSI-X functionality. It essentially goes through and tests
+ * various combinations of the set, clear, and mask bits in order to
+ * verify the behavior is as we expect it to be from the driver.
+ *
+ * Return: See enum fbnic_msix_self_test_codes
+ **/
+enum fbnic_msix_self_test_codes fbnic_msix_test(struct fbnic_dev *fbd)
+{
+	enum fbnic_msix_self_test_codes result = FBNIC_TEST_MSIX_SUCCESS;
+	struct pci_dev *pdev = to_pci_dev(fbd->dev);
+	struct fbnic_msix_test_data *test_data;
+	u32 mask = 0;
+	int i;
+
+	/* Allocate bitmap and IRQ vector table */
+	test_data = kzalloc(sizeof(*test_data), GFP_KERNEL);
+
+	/* memory allocation failure */
+	if (!test_data)
+		return FBNIC_TEST_MSIX_NOMEM;
+
+	/* Initialize test data */
+	test_data->fbd = fbd;
+
+	for (i = FBNIC_NON_NAPI_VECTORS; i < fbd->num_irqs; i++) {
+		/* Add IRQ to vector table so it can be found */
+		test_data->irq_vector[i] = pci_irq_vector(pdev, i);
+
+		/* Enable the interrupt */
+		if (!fbnic_request_irq(fbd, i, fbnic_irq_test, 0,
+				       fbd->netdev->name, test_data))
+			continue;
+
+		while (i-- > FBNIC_NON_NAPI_VECTORS)
+			fbnic_free_irq(fbd, i, test_data);
+		kfree(test_data);
+
+		/* IRQ request failure */
+		return FBNIC_TEST_MSIX_IRQ_REQ_FAIL;
+	}
+
+	/* Test each bit individually */
+	for (i = FBNIC_NON_NAPI_VECTORS; i < fbd->num_irqs; i++) {
+		mask = 1U << (i % 32);
+
+		/* Start with mask set and interrupt cleared */
+		fbnic_wr32(fbd, FBNIC_INTR_MASK_SET(i / 32), mask);
+		fbnic_wrfl(fbd);
+		fbnic_wr32(fbd, FBNIC_INTR_CLEAR(i / 32), mask);
+		fbnic_wrfl(fbd);
+
+		/* masking failure to prevent interrupt */
+		result = FBNIC_TEST_MSIX_MASK;
+
+		fbnic_wr32(fbd, FBNIC_INTR_SET(i / 32), mask);
+		fbnic_wrfl(fbd);
+		usleep_range(10000, 11000);
+
+		if (test_bit(i, test_data->test_msix_status))
+			break;
+
+		/* unmasking failure w/ sw status set */
+		result = FBNIC_TEST_MSIX_UNMASK;
+
+		fbnic_wr32(fbd, FBNIC_INTR_MASK_CLEAR(i / 32), mask);
+		fbnic_wrfl(fbd);
+		usleep_range(10000, 11000);
+
+		if (!test_bit(i, test_data->test_msix_status))
+			break;
+
+		/* interrupt when clearing mask */
+		result = FBNIC_TEST_MSIX_IRQ_CLEAR;
+
+		clear_bit(i, test_data->test_msix_status);
+		fbnic_wr32(fbd, FBNIC_INTR_MASK_CLEAR(i / 32), mask);
+		fbnic_wrfl(fbd);
+		usleep_range(10000, 11000);
+
+		if (test_bit(i, test_data->test_msix_status))
+			break;
+
+		/* interrupt not triggering when not masked */
+		result = FBNIC_TEST_MSIX_NO_INTERRUPT;
+
+		fbnic_wr32(fbd, FBNIC_INTR_SET(i / 32), mask);
+		fbnic_wrfl(fbd);
+		usleep_range(10000, 11000);
+
+		if (!test_bit(i, test_data->test_msix_status))
+			break;
+
+		/* status not cleared, or mask not set */
+		result = FBNIC_TEST_MSIX_NO_CLEAR_OR_MASK;
+		if (mask & fbnic_rd32(fbd, FBNIC_INTR_STATUS(i / 32)))
+			break;
+		if (!(mask & fbnic_rd32(fbd, FBNIC_INTR_MASK(i / 32))))
+			break;
+
+		/* Result = 0 - Success */
+		result = FBNIC_TEST_MSIX_SUCCESS;
+
+		clear_bit(i, test_data->test_msix_status);
+	}
+
+	if (i < fbd->num_irqs) {
+		fbnic_wr32(fbd, FBNIC_INTR_MASK_SET(i / 32), mask);
+		fbnic_wrfl(fbd);
+		fbnic_wr32(fbd, FBNIC_INTR_CLEAR(i / 32), mask);
+		fbnic_wrfl(fbd);
+		clear_bit(i, test_data->test_msix_status);
+	}
+
+	for (i = FBNIC_NON_NAPI_VECTORS; i < fbd->num_irqs; i++) {
+		/* Test for bits set after testing */
+		if (test_bit(i, test_data->test_msix_status))
+			result = 70;
+
+		/* Free IRQ */
+		fbnic_free_irq(fbd, i, test_data);
+	}
+
+	kfree(test_data);
+
+	return result;
+}
+
 void fbnic_napi_name_irqs(struct fbnic_dev *fbd)
 {
 	unsigned int i;
--
2.43.0

Re: [net-next,v2.2,3/5] eth fbnic: Add msix self test

[Jakub Kicinski]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
eth fbnic: Add msix self test

This function is meant to test the global interrupt registers and the
PCIe IP MSI-X functionality. It essentially goes through and tests
various combinations of the set, clear, and mask bits in order to
verify the behavior is as we expect it to be from the driver.

> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic.h b/drivers/net/ethernet/meta/fbnic/fbnic.h
> index 779a083b9215..092fa9ad93d0 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic.h
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic.h
> @@ -194,6 +194,36 @@ void fbnic_synchronize_irq(struct fbnic_dev *fbd, int nr);
>  int fbnic_request_irq(struct fbnic_dev *dev, int nr, irq_handler_t handler,
>  		      unsigned long flags, const char *name, void *data);
>  void fbnic_free_irq(struct fbnic_dev *dev, int nr, void *data);
> +
> +/**
> + * enum fbnic_msix_self_test_codes - return codes from self test routines
> + *
> + * These are the codes returned from the self test routines and
> + * stored in the test result array indexed by the specific
> + * test name.
> + *
> + * @FBNIC_TEST_MSIX_SUCCESS: no errors
> + * @FBNIC_TEST_MSIX_NOMEM: allocation failure
> + * @FBNIC_TEST_MSIX_IRQ_REQ_FAIL: IRQ request failure
> + * @FBNIC_TEST_MSIX_MASK: masking failed to prevent IRQ
> + * @FBNIC_TEST_MSIX_UNMASK: unmasking failure w/ sw status set
> + * @FBNIC_TEST_MSIX_IRQ_CLEAR: interrupt when clearing mask
> + * @FBNIC_TEST_MSIX_NO_INTERRUPT: no interrupt when not masked
> + * @FBNIC_TEST_MSIX_NO_CLEAR_OR_MASK: status not cleared, or mask not set
> + */
> +enum fbnic_msix_self_test_codes {
> +	FBNIC_TEST_MSIX_SUCCESS = 0,
> +	FBNIC_TEST_MSIX_NOMEM = 5,
> +	FBNIC_TEST_MSIX_IRQ_REQ_FAIL = 10,
> +	FBNIC_TEST_MSIX_MASK = 20,
> +	FBNIC_TEST_MSIX_UNMASK = 30,
> +	FBNIC_TEST_MSIX_IRQ_CLEAR = 40,
> +	FBNIC_TEST_MSIX_NO_INTERRUPT = 50,
> +	FBNIC_TEST_MSIX_NO_CLEAR_OR_MASK = 60,
> +};

[ ... ]

> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_irq.c b/drivers/net/ethernet/meta/fbnic/fbnic_irq.c
> index 02e8b0b257fe..2099ee89fb51 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic_irq.c
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic_irq.c

[ ... ]

> @@ -238,6 +238,160 @@ void fbnic_free_irq(struct fbnic_dev *fbd, int nr, void *data)
>  	free_irq(irq, data);
>  }
>
> +struct fbnic_msix_test_data {
> +	struct fbnic_dev *fbd;
> +	unsigned long test_msix_status[BITS_TO_LONGS(FBNIC_MAX_MSIX_VECS)];
> +	int irq_vector[FBNIC_MAX_MSIX_VECS];
> +};
> +
> +static irqreturn_t fbnic_irq_test(int irq, void *data)
> +{
> +	struct fbnic_msix_test_data *test_data = data;
> +	struct fbnic_dev *fbd = test_data->fbd;
> +	int i;
> +
> +	for (i = fbd->num_irqs; i--;) {
> +		if (test_data->irq_vector[i] == irq) {
> +			set_bit(i, test_data->test_msix_status);
> +			break;
> +		}
> +	}
> +
> +	return IRQ_HANDLED;
> +}
> +
> +/**
> + * fbnic_msix_test - Verify behavior of NIC interrupts
> + * @fbd: device to test
> + *
> + * This function is meant to test the global interrupt registers and the
> + * PCIe IP MSI-X functionality. It essentially goes through and tests
> + * various combinations of the set, clear, and mask bits in order to
> + * verify the behavior is as we expect it to be from the driver.
> + *
> + * Return: See enum fbnic_msix_self_test_codes
> + **/
> +enum fbnic_msix_self_test_codes fbnic_msix_test(struct fbnic_dev *fbd)
> +{
> +	enum fbnic_msix_self_test_codes result = FBNIC_TEST_MSIX_SUCCESS;
> +	struct pci_dev *pdev = to_pci_dev(fbd->dev);
> +	struct fbnic_msix_test_data *test_data;
> +	u32 mask = 0;
> +	int i;
> +
> +	/* Allocate bitmap and IRQ vector table */
> +	test_data = kzalloc(sizeof(*test_data), GFP_KERNEL);
> +
> +	/* memory allocation failure */
> +	if (!test_data)
> +		return FBNIC_TEST_MSIX_NOMEM;
> +
> +	/* Initialize test data */
> +	test_data->fbd = fbd;
> +
> +	for (i = FBNIC_NON_NAPI_VECTORS; i < fbd->num_irqs; i++) {
> +		/* Add IRQ to vector table so it can be found */
> +		test_data->irq_vector[i] = pci_irq_vector(pdev, i);
> +
> +		/* Enable the interrupt */
> +		if (!fbnic_request_irq(fbd, i, fbnic_irq_test, 0,
> +				       fbd->netdev->name, test_data))
> +			continue;
> +
> +		while (i-- > FBNIC_NON_NAPI_VECTORS)
> +			fbnic_free_irq(fbd, i, test_data);
> +		kfree(test_data);
> +
> +		/* IRQ request failure */
> +		return FBNIC_TEST_MSIX_IRQ_REQ_FAIL;
> +	}
> +
> +	/* Test each bit individually */
> +	for (i = FBNIC_NON_NAPI_VECTORS; i < fbd->num_irqs; i++) {
> +		mask = 1U << (i % 32);
> +
> +		/* Start with mask set and interrupt cleared */
> +		fbnic_wr32(fbd, FBNIC_INTR_MASK_SET(i / 32), mask);
> +		fbnic_wrfl(fbd);
> +		fbnic_wr32(fbd, FBNIC_INTR_CLEAR(i / 32), mask);
> +		fbnic_wrfl(fbd);
> +
> +		/* masking failure to prevent interrupt */
> +		result = FBNIC_TEST_MSIX_MASK;
> +
> +		fbnic_wr32(fbd, FBNIC_INTR_SET(i / 32), mask);
> +		fbnic_wrfl(fbd);
> +		usleep_range(10000, 11000);
> +
> +		if (test_bit(i, test_data->test_msix_status))
> +			break;
> +
> +		/* unmasking failure w/ sw status set */
> +		result = FBNIC_TEST_MSIX_UNMASK;
> +
> +		fbnic_wr32(fbd, FBNIC_INTR_MASK_CLEAR(i / 32), mask);
> +		fbnic_wrfl(fbd);
> +		usleep_range(10000, 11000);
> +
> +		if (!test_bit(i, test_data->test_msix_status))
> +			break;
> +
> +		/* interrupt when clearing mask */
> +		result = FBNIC_TEST_MSIX_IRQ_CLEAR;
> +
> +		clear_bit(i, test_data->test_msix_status);
> +		fbnic_wr32(fbd, FBNIC_INTR_MASK_CLEAR(i / 32), mask);
> +		fbnic_wrfl(fbd);
> +		usleep_range(10000, 11000);
> +
> +		if (test_bit(i, test_data->test_msix_status))
> +			break;
> +
> +		/* interrupt not triggering when not masked */
> +		result = FBNIC_TEST_MSIX_NO_INTERRUPT;
> +
> +		fbnic_wr32(fbd, FBNIC_INTR_SET(i / 32), mask);
> +		fbnic_wrfl(fbd);
> +		usleep_range(10000, 11000);
> +
> +		if (!test_bit(i, test_data->test_msix_status))
> +			break;
> +
> +		/* status not cleared, or mask not set */
> +		result = FBNIC_TEST_MSIX_NO_CLEAR_OR_MASK;
> +		if (mask & fbnic_rd32(fbd, FBNIC_INTR_STATUS(i / 32)))
> +			break;
> +		if (!(mask & fbnic_rd32(fbd, FBNIC_INTR_MASK(i / 32))))
> +			break;
> +
> +		/* Result = 0 - Success */
> +		result = FBNIC_TEST_MSIX_SUCCESS;
> +
> +		clear_bit(i, test_data->test_msix_status);
> +	}
> +
> +	if (i < fbd->num_irqs) {
> +		fbnic_wr32(fbd, FBNIC_INTR_MASK_SET(i / 32), mask);
> +		fbnic_wrfl(fbd);
> +		fbnic_wr32(fbd, FBNIC_INTR_CLEAR(i / 32), mask);
> +		fbnic_wrfl(fbd);
> +		clear_bit(i, test_data->test_msix_status);
> +	}
> +
> +	for (i = FBNIC_NON_NAPI_VECTORS; i < fbd->num_irqs; i++) {
> +		/* Test for bits set after testing */
> +		if (test_bit(i, test_data->test_msix_status))
> +			result = 70;
                    ^^

Should the value 70 have a named constant in enum fbnic_msix_self_test_codes?

The function returns enum fbnic_msix_self_test_codes, which defines values
SUCCESS (0), NOMEM (5), IRQ_REQ_FAIL (10), MASK (20), UNMASK (30),
IRQ_CLEAR (40), NO_INTERRUPT (50), and NO_CLEAR_OR_MASK (60).

The value 70 appears to indicate a stray interrupt was detected after
testing completed, but this error condition has no corresponding named enum
constant. This makes the error code returned to userspace via ethtool
undocumented and prevents matching it to a failure description.

Should a constant such as FBNIC_TEST_MSIX_STRAY_INTERRUPT be added to the
enum to maintain consistency with all other error codes in this function?

> +
> +		/* Free IRQ */
> +		fbnic_free_irq(fbd, i, test_data);
> +	}
> +
> +	kfree(test_data);
> +
> +	return result;
> +}
-- 
pw-bot: cr

[PATCH net-next v2.2 4/5] eth fbnic: TLV support for use by MBX self test

[mike.marciniszyn]

From: "Mike Marciniszyn (Meta)" <mike.marciniszyn@gmail.com>

The TLV (Type-Value-Length) self uses a known set of data to create a
TLV message.  These routines support the MBX self test by creating
the test messages and parsing the response message coming back
from the firmware.

Signed-off-by: Mike Marciniszyn (Meta) <mike.marciniszyn@gmail.com>
---
v2 - place forward of struct fbnic_dev after include with a blank line

 drivers/net/ethernet/meta/fbnic/fbnic_tlv.c | 276 ++++++++++++++++++++
 drivers/net/ethernet/meta/fbnic/fbnic_tlv.h |  27 ++
 2 files changed, 303 insertions(+)

diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_tlv.c b/drivers/net/ethernet/meta/fbnic/fbnic_tlv.c
index 517ed8b2f1cb..c55d4f76a5fc 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_tlv.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_tlv.c
@@ -551,6 +551,172 @@ int fbnic_tlv_parser_error(void *opaque, struct fbnic_tlv_msg **results)
 	return -EBADMSG;
 }

+#define FBNIC_TLV_TEST_STRING_LEN	32
+
+struct fbnic_tlv_test {
+	u64	test_u64;
+	s64	test_s64;
+	u32	test_u32;
+	s32	test_s32;
+	u16	test_u16;
+	s16	test_s16;
+	u8	test_mac[ETH_ALEN];
+	u8	test_mac_array[4][ETH_ALEN];
+	u8	test_true;
+	u8	test_false;
+	char	test_string[FBNIC_TLV_TEST_STRING_LEN];
+};
+
+static struct fbnic_tlv_test test_struct;
+
+const struct fbnic_tlv_index fbnic_tlv_test_index[] = {
+	FBNIC_TLV_ATTR_U64(FBNIC_TLV_TEST_MSG_U64),
+	FBNIC_TLV_ATTR_S64(FBNIC_TLV_TEST_MSG_S64),
+	FBNIC_TLV_ATTR_U32(FBNIC_TLV_TEST_MSG_U32),
+	FBNIC_TLV_ATTR_S32(FBNIC_TLV_TEST_MSG_S32),
+	FBNIC_TLV_ATTR_U32(FBNIC_TLV_TEST_MSG_U16),
+	FBNIC_TLV_ATTR_S32(FBNIC_TLV_TEST_MSG_S16),
+	FBNIC_TLV_ATTR_MAC_ADDR(FBNIC_TLV_TEST_MSG_MAC_ADDR),
+	FBNIC_TLV_ATTR_FLAG(FBNIC_TLV_TEST_MSG_FLAG_TRUE),
+	FBNIC_TLV_ATTR_FLAG(FBNIC_TLV_TEST_MSG_FLAG_FALSE),
+	FBNIC_TLV_ATTR_STRING(FBNIC_TLV_TEST_MSG_STRING,
+			      FBNIC_TLV_TEST_STRING_LEN),
+	FBNIC_TLV_ATTR_ARRAY(FBNIC_TLV_TEST_MSG_ARRAY),
+	FBNIC_TLV_ATTR_NESTED(FBNIC_TLV_TEST_MSG_NESTED),
+	FBNIC_TLV_ATTR_LAST
+};
+
+static void fbnic_tlv_test_struct_init(void)
+{
+	int i = FBNIC_TLV_TEST_STRING_LEN - 1;
+
+	/* Populate the struct with random data */
+	get_random_once(&test_struct,
+			offsetof(struct fbnic_tlv_test, test_string) + i);
+
+	/* Force true/false to their expected values */
+	test_struct.test_false = false;
+	test_struct.test_true = true;
+
+	/* Convert test_string to a true ASCII string */
+	test_struct.test_string[i] = '\0';
+	while (i--) {
+		/* Force characters into displayable range */
+		if (test_struct.test_string[i] < 64 ||
+		    test_struct.test_string[i] >= 96) {
+			test_struct.test_string[i] %= 32;
+			test_struct.test_string[i] += 64;
+		}
+	}
+}
+
+static int fbnic_tlv_test_attr_data(struct fbnic_tlv_msg *msg)
+{
+	struct fbnic_tlv_msg *array;
+	int err, i;
+
+	err = fbnic_tlv_attr_put_int(msg, FBNIC_TLV_TEST_MSG_U64,
+				     test_struct.test_u64);
+	if (err)
+		return err;
+
+	err = fbnic_tlv_attr_put_int(msg, FBNIC_TLV_TEST_MSG_S64,
+				     test_struct.test_s64);
+	if (err)
+		return err;
+
+	err = fbnic_tlv_attr_put_int(msg, FBNIC_TLV_TEST_MSG_U32,
+				     test_struct.test_u32);
+	if (err)
+		return err;
+
+	err = fbnic_tlv_attr_put_int(msg, FBNIC_TLV_TEST_MSG_S32,
+				     test_struct.test_s32);
+	if (err)
+		return err;
+
+	err = fbnic_tlv_attr_put_int(msg, FBNIC_TLV_TEST_MSG_U16,
+				     test_struct.test_u16);
+	if (err)
+		return err;
+
+	err = fbnic_tlv_attr_put_int(msg, FBNIC_TLV_TEST_MSG_S16,
+				     test_struct.test_s16);
+	if (err)
+		return err;
+
+	err = fbnic_tlv_attr_put_value(msg, FBNIC_TLV_TEST_MSG_MAC_ADDR,
+				       test_struct.test_mac, ETH_ALEN);
+	if (err)
+		return err;
+
+	/* Start MAC address array */
+	array = fbnic_tlv_attr_nest_start(msg, FBNIC_TLV_TEST_MSG_ARRAY);
+	if (!array)
+		return -ENOSPC;
+
+	for (i = 0; i < 4; i++) {
+		err = fbnic_tlv_attr_put_value(array,
+					       FBNIC_TLV_TEST_MSG_MAC_ADDR,
+					       test_struct.test_mac_array[i],
+					       ETH_ALEN);
+		if (err)
+			return err;
+	}
+
+	/* Close array */
+	fbnic_tlv_attr_nest_stop(msg);
+
+	err = fbnic_tlv_attr_put_flag(msg, FBNIC_TLV_TEST_MSG_FLAG_TRUE);
+	if (err)
+		return err;
+
+	return fbnic_tlv_attr_put_string(msg, FBNIC_TLV_TEST_MSG_STRING,
+					 test_struct.test_string);
+}
+
+/**
+ * fbnic_tlv_test_create - Allocate a test message and fill it w/ data
+ * @fbd: FBNIC device structure
+ *
+ * Return: NULL on failure to allocate or pointer to new TLV test message.
+ **/
+struct fbnic_tlv_msg *fbnic_tlv_test_create(struct fbnic_dev *fbd)
+{
+	struct fbnic_tlv_msg *msg, *nest;
+	int err;
+
+	msg = fbnic_tlv_msg_alloc(FBNIC_TLV_MSG_ID_TEST);
+	if (!msg)
+		return NULL;
+
+	/* Randomize struct data */
+	fbnic_tlv_test_struct_init();
+
+	/* Add first level of data to message */
+	err = fbnic_tlv_test_attr_data(msg);
+	if (err)
+		goto free_message;
+
+	/* Start second level nested */
+	nest = fbnic_tlv_attr_nest_start(msg, FBNIC_TLV_TEST_MSG_NESTED);
+	if (!nest)
+		goto free_message;
+
+	/* Add nested data */
+	err = fbnic_tlv_test_attr_data(nest);
+	if (err)
+		goto free_message;
+
+	/* Close nest and report full message */
+	fbnic_tlv_attr_nest_stop(msg);
+
+	return msg;
+free_message:
+	free_page((unsigned long)msg);
+	return NULL;
+}
+
 void fbnic_tlv_attr_addr_copy(u8 *dest, struct fbnic_tlv_msg *src)
 {
 	u8 *mac_addr;
@@ -558,3 +724,113 @@ void fbnic_tlv_attr_addr_copy(u8 *dest, struct fbnic_tlv_msg *src)
 	mac_addr = fbnic_tlv_attr_get_value_ptr(src);
 	memcpy(dest, mac_addr, ETH_ALEN);
 }
+
+/**
+ * fbnic_tlv_parser_test_attr - Function loading test attributes into structure
+ * @str: Test structure to load
+ * @results: Pointer to results array
+ *
+ * Copies attributes into structure. Any attribute that doesn't exist in the
+ * results array is not populated.
+ **/
+static void fbnic_tlv_parser_test_attr(struct fbnic_tlv_test *str,
+				       struct fbnic_tlv_msg **results)
+{
+	struct fbnic_tlv_msg *array_results[4];
+	struct fbnic_tlv_msg *attr;
+	char *string = NULL;
+	int i, err;
+
+	str->test_u64 = fta_get_uint(results, FBNIC_TLV_TEST_MSG_U64);
+	str->test_u32 = fta_get_uint(results, FBNIC_TLV_TEST_MSG_U32);
+	str->test_u16 = fta_get_uint(results, FBNIC_TLV_TEST_MSG_U16);
+
+	str->test_s64 = fta_get_sint(results, FBNIC_TLV_TEST_MSG_S64);
+	str->test_s32 = fta_get_sint(results, FBNIC_TLV_TEST_MSG_S32);
+	str->test_s16 = fta_get_sint(results, FBNIC_TLV_TEST_MSG_S16);
+
+	attr = results[FBNIC_TLV_TEST_MSG_MAC_ADDR];
+	if (attr)
+		fbnic_tlv_attr_addr_copy(str->test_mac, attr);
+
+	attr = results[FBNIC_TLV_TEST_MSG_ARRAY];
+	if (attr) {
+		int len = le16_to_cpu(attr->hdr.len) / sizeof(u32) - 1;
+
+		err = fbnic_tlv_attr_parse_array(&attr[1], len,
+						 array_results,
+						 fbnic_tlv_test_index,
+						 FBNIC_TLV_TEST_MSG_MAC_ADDR,
+						 4);
+		if (!err) {
+			for (i = 0; i < 4 && array_results[i]; i++)
+				fbnic_tlv_attr_addr_copy(str->test_mac_array[i],
+							 array_results[i]);
+		}
+	}
+
+	str->test_true = !!results[FBNIC_TLV_TEST_MSG_FLAG_TRUE];
+	str->test_false = !!results[FBNIC_TLV_TEST_MSG_FLAG_FALSE];
+
+	attr = results[FBNIC_TLV_TEST_MSG_STRING];
+	if (attr) {
+		string = fbnic_tlv_attr_get_value_ptr(attr);
+		strscpy(str->test_string, string, FBNIC_TLV_TEST_STRING_LEN);
+	}
+}
+
+static void fbnic_tlv_test_dump(struct fbnic_tlv_test *value, char *prefix)
+{
+	print_hex_dump(KERN_INFO, prefix, DUMP_PREFIX_OFFSET, 16, 1,
+		       value, sizeof(*value), true);
+}
+
+/**
+ * fbnic_tlv_parser_test - Function for parsing and testing test message
+ * @opaque: Unused value
+ * @results: Results of parser output
+ *
+ * Return: negative value on error, or 0 on success.
+ *
+ * Parses attributes to structures and compares the structure to the
+ * expected test value that should have been used to populate the message.
+ *
+ * Used to verify message generation and parser are working correctly.
+ **/
+int fbnic_tlv_parser_test(void *opaque, struct fbnic_tlv_msg **results)
+{
+	struct fbnic_tlv_msg *nest_results[FBNIC_TLV_RESULTS_MAX] = { 0 };
+	struct fbnic_tlv_test result_struct;
+	struct fbnic_tlv_msg *attr;
+	int err;
+
+	memset(&result_struct, 0, sizeof(result_struct));
+	fbnic_tlv_parser_test_attr(&result_struct, results);
+
+	if (memcmp(&test_struct, &result_struct, sizeof(test_struct))) {
+		fbnic_tlv_test_dump(&result_struct, "fbnic: found - ");
+		fbnic_tlv_test_dump(&test_struct, "fbnic: expected - ");
+		return -EINVAL;
+	}
+
+	attr = results[FBNIC_TLV_TEST_MSG_NESTED];
+	if (!attr)
+		return -EINVAL;
+
+	err = fbnic_tlv_attr_parse(&attr[1],
+				   le16_to_cpu(attr->hdr.len) / sizeof(u32) - 1,
+				   nest_results, fbnic_tlv_test_index);
+	if (err)
+		return err;
+
+	memset(&result_struct, 0, sizeof(result_struct));
+	fbnic_tlv_parser_test_attr(&result_struct, nest_results);
+
+	if (memcmp(&test_struct, &result_struct, sizeof(test_struct))) {
+		fbnic_tlv_test_dump(&result_struct, "fbnic: found - ");
+		fbnic_tlv_test_dump(&test_struct, "fbnic: expected - ");
+		return -EINVAL;
+	}
+
+	return 0;
+}
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_tlv.h b/drivers/net/ethernet/meta/fbnic/fbnic_tlv.h
index 3508b46ebdd0..9c4e4759394a 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_tlv.h
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_tlv.h
@@ -9,6 +9,8 @@
 #include <linux/const.h>
 #include <linux/types.h>

+struct fbnic_dev;
+
 #define FBNIC_TLV_MSG_ALIGN(len)	ALIGN(len, sizeof(u32))
 #define FBNIC_TLV_MSG_SIZE(len)		\
 		(FBNIC_TLV_MSG_ALIGN(len) / sizeof(u32))
@@ -153,6 +155,31 @@ int fbnic_tlv_parser_error(void *opaque, struct fbnic_tlv_msg **results);
 #define fta_get_str(_results, _id, _dst, _dstsize) \
 	fbnic_tlv_attr_get_string(_results[_id], _dst, _dstsize)

+#define FBNIC_TLV_MSG_ID_TEST	0
+
+enum fbnic_tlv_test_attr_id {
+	FBNIC_TLV_TEST_MSG_U64,
+	FBNIC_TLV_TEST_MSG_S64,
+	FBNIC_TLV_TEST_MSG_U32,
+	FBNIC_TLV_TEST_MSG_S32,
+	FBNIC_TLV_TEST_MSG_U16,
+	FBNIC_TLV_TEST_MSG_S16,
+	FBNIC_TLV_TEST_MSG_MAC_ADDR,
+	FBNIC_TLV_TEST_MSG_FLAG_TRUE,
+	FBNIC_TLV_TEST_MSG_FLAG_FALSE,
+	FBNIC_TLV_TEST_MSG_STRING,
+	FBNIC_TLV_TEST_MSG_NESTED,
+	FBNIC_TLV_TEST_MSG_ARRAY,
+	FBNIC_TLV_TEST_MSG_MAX
+};
+
+extern const struct fbnic_tlv_index fbnic_tlv_test_index[];
+struct fbnic_tlv_msg *fbnic_tlv_test_create(struct fbnic_dev *fbd);
+int fbnic_tlv_parser_test(void *opaque, struct fbnic_tlv_msg **results);
+
+#define FBNIC_TLV_MSG_TEST \
+	FBNIC_TLV_PARSER(TEST, fbnic_tlv_test_index, \
+			 fbnic_tlv_parser_test)
 #define FBNIC_TLV_MSG_ERROR \
 	FBNIC_TLV_PARSER(UNKNOWN, NULL, fbnic_tlv_parser_error)
 #endif /* _FBNIC_TLV_H_ */
--
2.43.0

[PATCH net-next v2.2 5/5] eth fbnic: Add mailbox self test

[mike.marciniszyn]

From: "Mike Marciniszyn (Meta)" <mike.marciniszyn@gmail.com>

The mailbox self test ensures the interface to and from
the firmware is healthy by sending a test message and
fielding the response from the firmware.

This patch uses the new completion API [1][2] that allocates a
completion structure, binds the completion to the TEST
message, and uses a new FW parsing routine that wraps the
completion processing around the TLV parser.

Link: https://patch.msgid.link/20250516164804.741348-1-lee@trager.us [1]
Link: https://patch.msgid.link/20260115003353.4150771-6-mohsin.bashr@gmail.com [2]

Signed-off-by: Mike Marciniszyn (Meta) <mike.marciniszyn@gmail.com>
---
v2 - add enum for test return codes

 drivers/net/ethernet/meta/fbnic/fbnic.h       |  14 ++-
 .../net/ethernet/meta/fbnic/fbnic_ethtool.c   |  15 +++
 drivers/net/ethernet/meta/fbnic/fbnic_fw.c    | 100 ++++++++++++++++++
 drivers/net/ethernet/meta/fbnic/fbnic_fw.h    |  27 +++++
 3 files changed, 142 insertions(+)

diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
index b0c8d1b069e2..251de64baa5a 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_ethtool.c
@@ -129,11 +129,13 @@ static const struct fbnic_stat fbnic_gstrings_xdp_stats[] = {
 enum fbnic_self_test_results {
 	TEST_REG = 0,
 	TEST_MSIX,
+	TEST_MBX,
 };

 static const char fbnic_gstrings_self_test[][ETH_GSTRING_LEN] = {
 	[TEST_REG]	= "Register test (offline)",
 	[TEST_MSIX]	= "MSI-X Interrupt test (offline)",
+	[TEST_MBX]      = "FW mailbox test (on/offline)",
 };

 #define FBNIC_TEST_LEN ARRAY_SIZE(fbnic_gstrings_self_test)
@@ -1525,11 +1527,24 @@ static int fbnic_ethtool_msix_test(struct net_device *netdev, u64 *data)
 	return !!*data;
 }

+static int fbnic_ethtool_mbx_self_test(struct net_device *netdev, u64 *data)
+{
+	struct fbnic_net *fbn = netdev_priv(netdev);
+	struct fbnic_dev *fbd = fbn->fbd;
+
+	*data = fbnic_fw_mbx_self_test(fbd);
+
+	return !!*data;
+}
+
 static void fbnic_self_test(struct net_device *netdev,
 			    struct ethtool_test *eth_test, u64 *data)
 {
 	bool if_running = netif_running(netdev);

+	if (fbnic_ethtool_mbx_self_test(netdev, &data[TEST_MBX]))
+		eth_test->flags |= ETH_TEST_FL_FAILED;
+
 	if (!(eth_test->flags & ETH_TEST_FL_OFFLINE)) {
 		data[TEST_REG] = 0;
 		data[TEST_MSIX] = 0;
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_fw.c b/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
index 1f0b6350bef4..c2bad51bdde6 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_fw.c
@@ -378,6 +378,37 @@ fbnic_fw_get_cmpl_by_type(struct fbnic_dev *fbd, u32 msg_type)
 	return cmpl_data;
 }

+/**
+ * fbnic_fw_xmit_test_msg - Create and transmit a test message to FW mailbox
+ * @fbd: FBNIC device structure
+ * @cmpl: fw completion struct
+ *
+ * Return: zero on success, negative value on failure
+ *
+ * Generates a single page mailbox test message and places it in the Tx
+ * mailbox queue. Expectation is that the FW will validate that the nested
+ * value matches the external values, and then will echo them back to us.
+ *
+ * Also sets a completion slot for use in the completion wait calls when
+ * the cmpl arg is non-NULL.
+ */
+int fbnic_fw_xmit_test_msg(struct fbnic_dev *fbd,
+			   struct fbnic_fw_completion *cmpl)
+{
+	struct fbnic_tlv_msg *test_msg;
+	int err;
+
+	test_msg = fbnic_tlv_test_create(fbd);
+	if (!test_msg)
+		return -ENOMEM;
+
+	err = fbnic_mbx_map_req_w_cmpl(fbd, test_msg, cmpl);
+	if (err)
+		free_page((unsigned long)test_msg);
+
+	return err;
+}
+
 /**
  * fbnic_fw_xmit_simple_msg - Transmit a simple single TLV message w/o data
  * @fbd: FBNIC device structure
@@ -1556,7 +1587,29 @@ int fbnic_fw_xmit_send_logs(struct fbnic_dev *fbd, bool enable,
 	return err;
 }

+static int
+fbnic_fw_parser_test(void *opaque, struct fbnic_tlv_msg **results)
+{
+	struct fbnic_fw_completion *cmpl;
+	struct fbnic_dev *fbd = opaque;
+	int err;
+
+	/* find cmpl */
+	cmpl = fbnic_fw_get_cmpl_by_type(fbd, FBNIC_TLV_MSG_ID_TEST);
+	if (!cmpl)
+		return -ENOSPC;
+
+	err = fbnic_tlv_parser_test(opaque, results);
+
+	cmpl->result = err;
+	complete(&cmpl->done);
+	fbnic_fw_put_cmpl(cmpl);
+
+	return err;
+}
+
 static const struct fbnic_tlv_parser fbnic_fw_tlv_parser[] = {
+	FBNIC_TLV_PARSER(TEST, fbnic_tlv_test_index, fbnic_fw_parser_test),
 	FBNIC_TLV_PARSER(FW_CAP_RESP, fbnic_fw_cap_resp_index,
 			 fbnic_fw_parse_cap_resp),
 	FBNIC_TLV_PARSER(OWNERSHIP_RESP, fbnic_ownership_resp_index,
@@ -1787,6 +1840,53 @@ void fbnic_mbx_flush_tx(struct fbnic_dev *fbd)
 	} while (time_is_after_jiffies(timeout));
 }

+/**
+ * fbnic_fw_mbx_self_test() - verify firmware interface
+ * @fbd: device to test
+ *
+ * This function tests the interfaces to/from the firmware.
+ *
+ * Return: See enum fbnic_fw_self_test_codes
+ **/
+enum fbnic_fw_self_test_codes fbnic_fw_mbx_self_test(struct fbnic_dev *fbd)
+{
+	enum fbnic_fw_self_test_codes err;
+	struct fbnic_fw_completion *cmpl;
+
+	/* Skip test if FW interface is not present */
+	if (!fbnic_fw_present(fbd))
+		return FBNIC_TEST_FW_NO_FIRMWARE;
+
+	cmpl = fbnic_fw_alloc_cmpl(FBNIC_TLV_MSG_ID_TEST);
+	if (!cmpl)
+		return FBNIC_TEST_FW_NO_CMPL;
+
+	/* Load a test message onto the FW mailbox interface
+	 * and arm the completion.
+	 */
+	err = fbnic_fw_xmit_test_msg(fbd, cmpl);
+	if (err) {
+		err = FBNIC_TEST_FW_NO_XMIT;
+		goto exit_free;
+	}
+
+	/* Verify we received a message back */
+	if (!fbnic_mbx_wait_for_cmpl(cmpl)) {
+		err = FBNIC_TEST_FW_NO_MSG;
+		goto exit_cleanup;
+	}
+
+	/* Verify there were no parsing errors */
+	if (cmpl->result)
+		err = FBNIC_TEST_FW_PARSE;
+exit_cleanup:
+	fbnic_mbx_clear_cmpl(fbd, cmpl);
+exit_free:
+	fbnic_fw_put_cmpl(cmpl);
+
+	return err;
+}
+
 int fbnic_fw_xmit_rpc_macda_sync(struct fbnic_dev *fbd)
 {
 	struct fbnic_tlv_msg *mac_array;
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_fw.h b/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
index 8f7218900562..d84723e4cfa3 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_fw.h
@@ -104,6 +104,33 @@ void fbnic_mbx_clear_cmpl(struct fbnic_dev *fbd,
 void fbnic_mbx_poll(struct fbnic_dev *fbd);
 int fbnic_mbx_poll_tx_ready(struct fbnic_dev *fbd);
 void fbnic_mbx_flush_tx(struct fbnic_dev *fbd);
+
+/**
+ * enum fbnic_fw_self_test_codes - return codes from self test routines
+ *
+ * These are the codes returned from the self test routines and
+ * stored in the test result array indexed by the specific
+ * test name.
+ *
+ * @FBNIC_TEST_FW_SUCCESS: test success
+ * @FBNIC_TEST_FW_NO_FIRMWARE: FW interface not present
+ * @FBNIC_TEST_FW_NO_CMPL: No completion available
+ * @FBNIC_TEST_FW_NO_XMIT: Could not xmit message
+ * @FBNIC_TEST_FW_NO_MSG: no message returned
+ * @FBNIC_TEST_FW_PARSE: returned message had parsing error
+ */
+enum fbnic_fw_self_test_codes {
+	FBNIC_TEST_FW_SUCCESS = 0,
+	FBNIC_TEST_FW_NO_FIRMWARE = 10,
+	FBNIC_TEST_FW_NO_CMPL = 20,
+	FBNIC_TEST_FW_NO_XMIT = 30,
+	FBNIC_TEST_FW_NO_MSG = 40,
+	FBNIC_TEST_FW_PARSE = 50,
+};
+
+enum fbnic_fw_self_test_codes fbnic_fw_mbx_self_test(struct fbnic_dev *fbd);
+int fbnic_fw_xmit_test_msg(struct fbnic_dev *fbd,
+			   struct fbnic_fw_completion *c);
 int fbnic_fw_xmit_ownership_msg(struct fbnic_dev *fbd, bool take_ownership);
 int fbnic_fw_init_heartbeat(struct fbnic_dev *fbd, bool poll);
 void fbnic_fw_check_heartbeat(struct fbnic_dev *fbd);
--
2.43.0