Re: [PATCH net-next v3 3/5] eth fbnic: Add msix self test

[Mike Marciniszyn]

On Thu, Mar 05, 2026 at 10:09:45AM -0500, mike.marciniszyn@xxxxxxxxx wrote:
> From: "Mike Marciniszyn (Meta)" <mike.marciniszyn@xxxxxxxxx>
>
> 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@xxxxxxxxx>
> ---
> v2 - add enum for test return codes
> v3 - add missing enum
>
>  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, 214 insertions(+)
>
> diff --git a/drivers/net/ethernet/meta/fbnic/fbnic.h b/drivers/net/ethernet/meta/fbnic/fbnic.h
> index 779a083b9215..534caa3c872d 100644
> --- a/drivers/net/ethernet/meta/fbnic/fbnic.h
> +++ b/drivers/net/ethernet/meta/fbnic/fbnic.h
> @@ -194,6 +194,38 @@ 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
> + * @FBNIC_TEST_MSIX_BITS_SET_AFTER_TEST: Bits are set after test
> + */
> +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,
> +	FBNIC_TEST_MSIX_BITS_SET_AFTER_TEST = 70,
> +};
> +
> +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..44034d53112c 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);

There is a change in the patchworks checkpatch test the wants to see this as:

test_data = kzalloc_obj(*test_data, GFP_KERNEL);

Unfortunately, the net-next repo does not yet have this api and doesn't
have the checkpatch and the new api that comes in:

Commit 070580b0b174 ("checkpatch: Suggest kmalloc_obj family for sizeof allocations")

This looks like the checkpatch.pl in the test is not from net-next/main.

> +
> +	/* 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 = FBNIC_TEST_MSIX_BITS_SET_AFTER_TEST;
> +
> +		/* 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
>