[PATCH v2 00/10] selftests/vfio: igb: Add driver for Intel

[PATCH v2 00/10] selftests/vfio: igb: Add driver for Intel

[Josh Hilke]

This is v2 of a series which adds VFIO selftest driver for the Intel
82576 Ethernet Gigabit Controller (IGB). This device is emulated in QEMU
and is also ubiquitous on many systems which makes it a good candidate
device for running VFIO selftests.

v1: https://lore.kernel.org/kvm/20260511211839.2781731-1-jrhilke@google.com/

Major changes in v2
===================
1. Patches 2 through 9 add Alex Williamson's series which adds support
for the physical device since v1 only supported the QEMU-emulated
device. 

https://lore.kernel.org/kvm/20260515220330.565792-1-alex.williamson@nvidia.com/

2. Patch 10 adds the official IGB header files to the driver, which
eliminates duplicate register definitions.

Testing
=======
- VFIO selftests builds clean at every commit
- vfio_pci_driver_test passes using IGB driver + QEMU. Use the following
  VNG command to run the tests:

  vng \
  --run arch/x86/boot/bzImage \
  --user root \
  --disable-microvm \
  --memory 32G \
  --cpus 8 \
  --qemu-opts="-M q35,accel=kvm,kernel-irqchip=split" \
  --qemu-opts="-device intel-iommu,intremap=on,caching-mode=on,device-iotlb=on" \
  --qemu-opts="-netdev user,id=net0 -device igb,netdev=net0,addr=09.0" \
  --append "console=ttyS0 earlyprintk=ttyS0 intel_iommu=on iommu=pt" \
  --exec "modprobe vfio-pci && \
          ./tools/testing/selftests/vfio/scripts/setup.sh 0000:00:09.0 && \
          ./tools/testing/selftests/vfio/scripts/run.sh ./tools/testing/selftests/vfio/vfio_pci_driver_test"

Future work
===========
Modify QEMU's implementation of IGB loopback data transfer to be
asynchronous. Alex noted that QEMU's emulation of transmitting data in
loopback mode is synchronous, i.e by the time memcpy_start() returns,
the data is already in the Rx queue. This won't work for testing
continuous DMA (memcpy) across Live Update + kexec.

One solution is to simulate the line rate in QEMU. This would allow us
to slow down the rate of DMA, so that it continues for the duration of
Live Update.

Assistance
==========
- Claude Opus 4.7 with additional assistance from GPT 5.5 for Alex's
  patches
- Gemini 3.1 Pro Preview for my patches

--------
Changelog:

v1 -> v2
- Removed the chunking loop in igb_memcpy_start() (David)
- Removed redundant writes to status_error and hdr_addr (David)
- Include official IGB header files (David)

Alex Williamson (8):
  selftests/vfio: igb: Use PHY internal loopback on 82576
  selftests/vfio: igb: Use advanced TX and RX descriptors
  selftests/vfio: igb: Program MSI-X interrupt routing
  selftests/vfio: igb: Extend memcpy completion timeout for line-rate
    hardware
  selftests/vfio: igb: Disable PCIe completion timeout retries
  selftests/vfio: Add vfio_pci_irq_reenable() helper
  selftests/vfio: igb: Factor hardware programming into igb_hw_init()
  selftests/vfio: igb: Recover after DMA-read faults

Josh Hilke (2):
  selftests/vfio: igb: Add driver for IGB QEMU device
  selftests/vfio: igb: Use offical IGB headers in selftest driver

 .../selftests/vfio/lib/drivers/igb/igb.c      | 524 ++++++++++++++++++
 .../lib/include/libvfio/vfio_pci_device.h     |   2 +
 tools/testing/selftests/vfio/lib/libvfio.mk   |   2 +
 .../selftests/vfio/lib/vfio_pci_device.c      |  22 +
 .../selftests/vfio/lib/vfio_pci_driver.c      |   2 +
 5 files changed, 552 insertions(+)
 create mode 100644 tools/testing/selftests/vfio/lib/drivers/igb/igb.c

-- 
2.54.0.794.g4f17f83d09-goog

[PATCH v2 01/10] selftests/vfio: igb: Add driver for IGB QEMU device

[Josh Hilke]

Add a VFIO selftest driver for the Intel Gigabit Ethernet controller
(IGB). IGB is fully virtualized in QEMU [1] which makes it easy to run
VFIO selftests without needing any specific hardware.

IGB does not have a default memcpy operation, and memcpy is required for
all VFIO selftest drivers. However, IGB has a "loopback" mode which is
used in this driver to implement memcpy. When IGB is in loopback mode,
it doesn't send data out to the network. Instead, it sends data from the
Tx queue directly to the Rx queue which points to the memcpy
destination, instead of sending the data out to the network.

This driver passes all of the VFIO selftests in
tools/testing/selftests/vfio/ when running in QEMU. The driver has not
been tested using a real IGB device since the main goal of writing this
driver is to run VFIO selftests in QEMU without requiring any hardware.

This command is used to test the driver. It runs
./tools/testing/selftests/vfio/vfio_pci_driver_test using virtme-ng [2],
which runs a kernel in a virtualized environment using QEMU that has a
copy-on-write snapshot the host filesystem.

vng \
  --run arch/x86/boot/bzImage \
  --user root \
  --disable-microvm \
  --memory 32G \
  --cpus 8 \
  --qemu-opts="-M q35,accel=kvm,kernel-irqchip=split" \
  --qemu-opts="-device intel-iommu,intremap=on,caching-mode=on,device-iotlb=on" \
  --qemu-opts="-netdev user,id=net0 -device igb,netdev=net0,addr=09.0" \
  --append "console=ttyS0 earlyprintk=ttyS0 intel_iommu=on iommu=pt" \
  --exec "modprobe vfio-pci && \
          ./tools/testing/selftests/vfio/scripts/setup.sh 0000:00:09.0 && \
          ./tools/testing/selftests/vfio/scripts/run.sh ./tools/testing/selftests/vfio/vfio_pci_driver_test"

Code was written entirely by AI (Gemini) by feeding it the Intel IGB
specification, the code for the real IGB driver, and the QEMU
implementation of the IGB device. It took many iterations of prompting
to get the driver to pass all of the tests, and lot's of de-slopping to
remove unnecessary code and make the code readable.

Code comments are also written by Gemini through iterative
prompting.

This patch is based on the kvm/queue branch.

[1] https://www.qemu.org/docs/master/system/devices/igb.html
[2] https://github.com/arighi/virtme-ng

Assisted-by: Gemini:gemini-3-pro-preview
Signed-off-by: Josh Hilke <jrhilke@google.com>
---
 .../selftests/vfio/lib/drivers/igb/igb.c      | 360 ++++++++++++++++++
 .../vfio/lib/drivers/igb/registers.h          | 103 +++++
 tools/testing/selftests/vfio/lib/libvfio.mk   |   1 +
 .../selftests/vfio/lib/vfio_pci_driver.c      |   2 +
 4 files changed, 466 insertions(+)
 create mode 100644 tools/testing/selftests/vfio/lib/drivers/igb/igb.c
 create mode 100644 tools/testing/selftests/vfio/lib/drivers/igb/registers.h

diff --git a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
new file mode 100644
index 000000000000..064ca7c05da0
--- /dev/null
+++ b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
@@ -0,0 +1,360 @@
+// SPDX-License-Identifier: GPL-2.0-only
+#include <unistd.h>
+#include <errno.h>
+#include <stdint.h>
+#include <linux/io.h>
+#include <linux/pci_regs.h>
+#include <linux/pci_ids.h>
+#include <libvfio/vfio_pci_device.h>
+
+#include "registers.h"
+
+#define PCI_DEVICE_ID_INTEL_82576 0x10C9
+#define IGB_MAX_CHUNK_SIZE 1024
+#define MSIX_VECTOR 0
+#define RING_SIZE 4096 /* Number of descriptors in ring */
+
+struct igb_tx_desc {
+	union {
+		struct {
+			u64 buffer_addr; /* Address of descriptor's data buffer */
+			u32 cmd_type_len; /* Command/Type/Length */
+			u32 olinfo_status; /* Context/Buffer info */
+		} read;
+
+		struct {
+			u64 rsvd;        /* Reserved */
+			u32 nxtseq_seed; /* Next sequence seed */
+			u32 status;      /* Descriptor status */
+		} wb;
+	};
+};
+
+struct igb_rx_desc {
+	union {
+		struct {
+			u64 pkt_addr; /* Packet buffer address */
+			u64 hdr_addr; /* Header buffer address */
+		} read;
+		struct {
+			u16 pkt_info;     /* RSS type, Packet type */
+			u16 hdr_info;     /* Split Head, buf len */
+			u32 rss;          /* RSS Hash */
+			u32 status_error; /* ext status/error */
+			u16 length;       /* Packet length */
+			u16 vlan;         /* VLAN tag */
+		} wb; /* writeback */
+	};
+};
+
+struct igb {
+	void *bar0;
+	u32 tx_tail;
+	u32 rx_tail;
+	struct igb_tx_desc tx_ring[RING_SIZE] __attribute__((aligned(128)));
+	struct igb_rx_desc rx_ring[RING_SIZE] __attribute__((aligned(128)));
+};
+
+static inline struct igb *to_igb_state(struct vfio_pci_device *device)
+{
+	return (struct igb *)device->driver.region.vaddr;
+}
+
+static inline void igb_write32(struct igb *igb, u32 reg, u32 val)
+{
+	writel(val, igb->bar0 + reg);
+}
+
+static inline u32 igb_read32(struct igb *igb, u32 reg)
+{
+	return readl(igb->bar0 + reg);
+}
+
+static int igb_write_phy(struct igb *igb, u32 offset, u16 data)
+{
+	u32 mdic;
+	int i;
+
+	mdic = (((u32)data) |
+		(offset << IGB_MDIC_REG_SHIFT) |
+		(1 << IGB_MDIC_PHY_SHIFT) |
+		IGB_MDIC_OP_WRITE);
+
+	igb_write32(igb, IGB_MDIC, mdic);
+
+	for (i = 0; i < 1000; i++) {
+		usleep(50);
+		mdic = igb_read32(igb, IGB_MDIC);
+		if (mdic & IGB_MDIC_READY)
+			break;
+	}
+
+	if (!(mdic & IGB_MDIC_READY))
+		return -1;
+
+	if (mdic & IGB_MDIC_ERROR)
+		return -1;
+
+	return 0;
+}
+
+static int igb_read_phy(struct igb *igb, u32 offset, u16 *data)
+{
+	u32 mdic;
+	int i;
+
+	mdic = ((offset << IGB_MDIC_REG_SHIFT) |
+		(1 << IGB_MDIC_PHY_SHIFT) |
+		IGB_MDIC_OP_READ);
+
+	igb_write32(igb, IGB_MDIC, mdic);
+
+	for (i = 0; i < 1000; i++) {
+		usleep(50);
+		mdic = igb_read32(igb, IGB_MDIC);
+		if (mdic & IGB_MDIC_READY)
+			break;
+	}
+
+	if (!(mdic & IGB_MDIC_READY))
+		return -1;
+
+	if (mdic & IGB_MDIC_ERROR)
+		return -1;
+
+	*data = (u16)mdic;
+	return 0;
+}
+
+static int igb_phy_setup_autoneg(struct igb *igb)
+{
+	int timeout_ms = 1000;
+	bool success = false;
+	u16 phy_status;
+	int ret;
+	int i;
+
+	/* Trigger auto-negotiation */
+	ret = igb_write_phy(igb, IGB_PHY_CTRL_REG_OFFSET,
+			    IGB_PHY_CTRL_AN_ENABLE | IGB_PHY_CTRL_RESTART_AN);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < timeout_ms; i++) {
+		if (igb_read_phy(igb, IGB_PHY_STATUS_REG_OFFSET, &phy_status) == 0) {
+			success = !!(phy_status & IGB_PHY_STATUS_AN_COMP);
+			if (success)
+				break;
+		}
+		usleep(1000);
+	}
+
+	if (!success) {
+		printf("igb: Auto-negotiation did not complete in time\n");
+		return -ETIMEDOUT;
+	}
+
+	return 0;
+}
+
+static int igb_probe(struct vfio_pci_device *device)
+{
+	if (!vfio_pci_device_match(device, PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_INTEL_82576))
+		return -EINVAL;
+
+	return 0;
+}
+
+static void igb_init(struct vfio_pci_device *device)
+{
+	struct igb *igb = to_igb_state(device);
+	u64 iova_tx, iova_rx;
+	u32 ctrl, rctl;
+	u16 cmd_reg;
+	int retries;
+
+	VFIO_ASSERT_GE(device->driver.region.size, sizeof(struct igb));
+
+	/* Set up rings and calculate IOVAs */
+	igb->bar0 = device->bars[0].vaddr;
+
+	iova_tx = to_iova(device, igb->tx_ring);
+	iova_rx = to_iova(device, igb->rx_ring);
+
+	/* Reset device and disable all interrupts */
+	igb_write32(igb, IGB_CTRL, igb_read32(igb, IGB_CTRL) | IGB_CTRL_RST);
+	usleep(20000);
+	igb_write32(igb, IGB_IMC, 0xFFFFFFFF);
+
+	/* Signal that the driver is loaded */
+	ctrl = igb_read32(igb, IGB_CTRL_EXT);
+	ctrl |= IGB_CTRL_EXT_DRV_LOAD;
+	ctrl &= ~IGB_CTRL_EXT_LINK_MODE_MASK;
+	igb_write32(igb, IGB_CTRL_EXT, ctrl);
+
+	/* Enable PCI Bus Master. */
+	cmd_reg = vfio_pci_config_readw(device, PCI_COMMAND);
+	if (!(cmd_reg & PCI_COMMAND_MASTER)) {
+		cmd_reg |= (PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY);
+		vfio_pci_config_writew(device, PCI_COMMAND, cmd_reg);
+	}
+
+	/* Trigger autonegotiation. This enables IGB to transmit data. */
+	if (igb_phy_setup_autoneg(igb))
+		return;
+
+	/* Configure TX and RX descriptor rings */
+	igb_write32(igb, IGB_TDBAL0, (u32)iova_tx);
+	igb_write32(igb, IGB_TDBAH0, (u32)(iova_tx >> 32));
+	igb_write32(igb, IGB_TDLEN0, RING_SIZE * sizeof(struct igb_tx_desc));
+	igb_write32(igb, IGB_TDH0, 0);
+	igb_write32(igb, IGB_TDT0, 0);
+	igb_write32(igb, IGB_TXDCTL0, IGB_TXDCTL0_Q_EN);
+
+	igb_write32(igb, IGB_RDBAL0, (u32)iova_rx);
+	igb_write32(igb, IGB_RDBAH0, (u32)(iova_rx >> 32));
+	igb_write32(igb, IGB_RDLEN0, RING_SIZE * sizeof(struct igb_rx_desc));
+	igb_write32(igb, IGB_RDH0, 0);
+	igb_write32(igb, IGB_RDT0, 0);
+	igb_write32(igb, IGB_RXDCTL0, IGB_RXDCTL0_Q_EN);
+
+	/* Wait for TX and RX queues to be enabled */
+	retries = 2000;
+	while (retries-- > 0) {
+		if ((igb_read32(igb, IGB_TXDCTL0) & IGB_TXDCTL0_Q_EN) &&
+		    (igb_read32(igb, IGB_RXDCTL0) & IGB_RXDCTL0_Q_EN))
+			break;
+		usleep(10);
+	}
+
+	/* Enable Receiver and Transmitter */
+	rctl = IGB_RCTL_EN |       /* Receiver Enable */
+	       IGB_RCTL_UPE |      /* Unicast Promiscuous (for dummy MAC) */
+	       IGB_RCTL_LBM_MAC |  /* MAC Loopback Mode */
+	       IGB_RCTL_SECRC;     /* Strip CRC (needed for memcmp) */
+	igb_write32(igb, IGB_RCTL, rctl);
+	igb_write32(igb, IGB_TCTL, IGB_TCTL_EN);
+
+	/* Enable MSI-X with 1 vector for the test */
+	vfio_pci_msix_enable(device, MSIX_VECTOR, 1);
+
+	/* Enable auto-masking of interrupts to avoid storms without a real ISR */
+	igb_write32(igb, IGB_GPIE, IGB_GPIE_EIAME);
+
+	/* Enable interrupts on vector 0 */
+	igb_write32(igb, IGB_EIMS, 1);
+
+	/* Map vector 0 to interrupt cause 0 and mark it valid */
+	igb_write32(igb, IGB_IVAR0, IGB_IVAR_VALID);
+
+	/* Initialize driver state and capability limits */
+	igb->tx_tail = 0;
+	igb->rx_tail = 0;
+
+	device->driver.max_memcpy_size = IGB_MAX_CHUNK_SIZE;
+	device->driver.max_memcpy_count = RING_SIZE - 1;
+	device->driver.msi = MSIX_VECTOR;
+}
+
+
+static void igb_remove(struct vfio_pci_device *device)
+{
+	struct igb *igb = to_igb_state(device);
+
+	vfio_pci_msix_disable(device);
+	igb_write32(igb, IGB_RCTL, 0);
+	igb_write32(igb, IGB_TCTL, 0);
+	igb_write32(igb, IGB_CTRL, igb_read32(igb, IGB_CTRL) | IGB_CTRL_RST);
+}
+
+static void igb_irq_disable(struct igb *igb)
+{
+	igb_write32(igb, IGB_EIMC, 1);
+}
+
+static void igb_irq_enable(struct igb *igb)
+{
+	igb_write32(igb, IGB_EIMS, 1);
+}
+
+static void igb_irq_clear(struct igb *igb)
+{
+	igb_read32(igb, IGB_EICR);
+}
+
+static void igb_memcpy_start(struct vfio_pci_device *device, iova_t src,
+			     iova_t dst, u64 size, u64 count)
+{
+	struct igb *igb = to_igb_state(device);
+	struct igb_rx_desc *rx;
+	struct igb_tx_desc *tx;
+	u32 i;
+
+	igb_irq_disable(igb);
+
+	for (i = 0; i < count; i++) {
+		tx = &igb->tx_ring[igb->tx_tail];
+		rx = &igb->rx_ring[igb->rx_tail];
+
+		memset(tx, 0, sizeof(struct igb_tx_desc));
+		memset(rx, 0, sizeof(struct igb_rx_desc));
+
+		rx->read.pkt_addr = dst;
+		tx->read.buffer_addr = src;
+		tx->read.cmd_type_len = (u32)size;
+		tx->read.cmd_type_len |= (u32)(IGB_TXD_CMD_EOP) << IGB_TXD_CMD_SHIFT;
+
+		/* Set to 0 to disable offloads and avoid needing a context descriptor */
+		tx->read.olinfo_status = 0;
+
+		igb->tx_tail = (igb->tx_tail + 1) % RING_SIZE;
+		igb->rx_tail = (igb->rx_tail + 1) % RING_SIZE;
+	}
+
+	igb_write32(igb, IGB_RDT0, igb->rx_tail);
+	igb_write32(igb, IGB_TDT0, igb->tx_tail);
+}
+
+static int igb_memcpy_wait(struct vfio_pci_device *device)
+{
+	struct igb *igb = to_igb_state(device);
+	struct igb_rx_desc *rx;
+	u32 prev_tail;
+	int retries;
+
+	prev_tail = (igb->rx_tail + RING_SIZE - 1) % RING_SIZE;
+	rx = &igb->rx_ring[prev_tail];
+
+	retries = 100;
+	while (retries-- > 0) {
+		if (rx->wb.status_error & 1)
+			break;
+		usleep(10);
+	}
+
+	igb_irq_clear(igb);
+
+	igb_irq_enable(igb);
+
+	if (rx->wb.status_error & 1)
+		return 0;
+
+	return -ETIMEDOUT;
+}
+
+static void igb_send_msi(struct vfio_pci_device *device)
+{
+	struct igb *igb = to_igb_state(device);
+
+	igb_write32(igb, IGB_EICS, 1);
+}
+
+const struct vfio_pci_driver_ops igb_ops = {
+	.name = "igb",
+	.probe = igb_probe,
+	.init = igb_init,
+	.remove = igb_remove,
+	.memcpy_start = igb_memcpy_start,
+	.memcpy_wait = igb_memcpy_wait,
+	.send_msi = igb_send_msi,
+};
diff --git a/tools/testing/selftests/vfio/lib/drivers/igb/registers.h b/tools/testing/selftests/vfio/lib/drivers/igb/registers.h
new file mode 100644
index 000000000000..e2756be2e9c0
--- /dev/null
+++ b/tools/testing/selftests/vfio/lib/drivers/igb/registers.h
@@ -0,0 +1,103 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#ifndef _IGB_REGISTERS_H_
+#define _IGB_REGISTERS_H_
+
+/* Register Offsets (Intel 82576EB Datasheet) */
+#define IGB_CTRL 0x00000 /* Device Control */
+#define IGB_STATUS 0x00008 /* Device Status */
+#define IGB_CTRL_EXT 0x00018 /* Extended Device Control */
+#define IGB_MDIC 0x00020 /* MDI Control */
+#define IGB_RCTL 0x00100 /* Receive Control */
+#define IGB_TCTL 0x00400 /* Transmit Control */
+#define IGB_SCTL 0x00420 /* SerDes Control */
+
+/* Interrupt Registers */
+#define IGB_IMC 0x0150C /* Interrupt Mask Clear */
+#define IGB_IVAR0 0x01700 /* Interrupt Vector Allocation Register 0 */
+
+/* Rx Ring 0 Registers */
+#define IGB_RDBAL0 0x0C000 /* Rx Desc Base Address Low */
+#define IGB_RDBAH0 0x0C004 /* Rx Desc Base Address High */
+#define IGB_RDLEN0 0x0C008 /* Rx Desc Length */
+#define IGB_RDH0 0x0C010 /* Rx Desc Head */
+#define IGB_RDT0 0x0C018 /* Rx Desc Tail */
+#define IGB_RXDCTL0 0x0C028 /* Rx Desc Control */
+
+/* Tx Ring 0 Registers */
+#define IGB_TDBAL0 0x0E000 /* Tx Desc Base Address Low */
+#define IGB_TDBAH0 0x0E004 /* Tx Desc Base Address High */
+#define IGB_TDLEN0 0x0E008 /* Tx Desc Length */
+#define IGB_TDH0 0x0E010 /* Tx Desc Head */
+#define IGB_TDT0 0x0E018 /* Tx Desc Tail */
+#define IGB_TXDCTL0 0x0E028 /* Tx Desc Control */
+
+/* Control Bit Definitions */
+/* CTRL */
+#define IGB_CTRL_RST (1 << 26) /* Device Reset */
+#define IGB_CTRL_EXT_LINK_MODE_MASK (3 << 22)
+
+/* CTRL_EXT */
+#define IGB_CTRL_EXT_DRV_LOAD (1 << 28) /* Driver Loaded */
+
+/* RCTL */
+#define IGB_RCTL_EN (1 << 1) /* Receiver Enable */
+#define IGB_RCTL_UPE (1 << 3) /* Unicast Promiscuous Enabled */
+#define IGB_RCTL_LPE (1 << 5) /* Long Packet Reception Enable */
+#define IGB_RCTL_LBM_MAC (1 << 6) /* Loopback Mode - MAC */
+#define IGB_RCTL_SECRC (1 << 26) /* Strip Ethernet CRC */
+
+/* TCTL */
+#define IGB_TCTL_EN (1 << 1) /* Transmit Enable */
+
+/* SCTL */
+#define IGB_SCTL_DISABLE_SERDES_LOOPBACK (1 << 6)
+
+/* MDIC */
+#define IGB_MDIC_OP_WRITE (1 << 26)
+#define IGB_MDIC_OP_READ  (2 << 26)
+
+#define IGB_EICR 0x01580 /* Extended Interrupt Cause Read */
+
+#define IGB_RAH0 0x05404 /* Receive Address High 0 */
+#define IGB_VMOLR0 0x05AD0 /* VM Offload Layout Register 0 */
+
+#define IGB_VMOLR_LPE 0x00010000 /* Long Packet Enable */
+#define IGB_VMOLR_BAM 0x08000000 /* Broadcast Accept Mode */
+#define IGB_RAH_POOL_1 0x00040000 /* Pool 1 assignment */
+
+#define IGB_EIMS 0x01524 /* Extended Interrupt Mask Set */
+#define IGB_EICS 0x01520 /* Extended Interrupt Cause Set */
+#define IGB_EIMC 0x01528 /* Extended Interrupt Mask Clear */
+#define IGB_CTRL_GIO_MASTER_DISABLE (1 << 2) /* GIO Master Disable */
+#define IGB_STATUS_GIO_MASTER_ENABLE (1 << 19) /* GIO Master Enable */
+#define IGB_GPIE 0x01514 /* General Purpose Interrupt Enable */
+#define IGB_TXDCTL0_Q_EN (1 << 25) /* Transmit Queue Enable */
+#define IGB_RXDCTL0_Q_EN (1 << 25) /* Receive Queue Enable */
+#define IGB_MRQC 0x05818 /* Multiple Receive Queues Command */
+
+#define IGB_MDIC_PHY_SHIFT 21 /* PHY Address Shift */
+#define IGB_MDIC_REG_SHIFT 16 /* Register Address Shift */
+#define IGB_MDIC_READY (1 << 28) /* MDI Data Ready */
+#define IGB_MDIC_ERROR (1 << 29) /* MDI Error */
+
+#define IGB_PHY_CTRL_REG_OFFSET 0
+#define IGB_PHY_STATUS_REG_OFFSET 1
+#define IGB_PHY_STATUS_AN_COMP 0x0020 /* Auto-negotiation complete */
+#define IGB_PHY_CTRL_AN_ENABLE 0x1000 /* Auto-Negotiation Enable */
+#define IGB_PHY_CTRL_RESTART_AN 0x0200 /* Restart Auto-Negotiation */
+
+#define IGB_PHY_AUTONEG_ADV_REG_OFFSET 4 /* Auto-Negotiation Advertisement */
+#define IGB_PHY_1000T_CTRL_REG_OFFSET 9 /* 1000Base-T Control */
+#define IGB_CR_1000T_FD_CAPS 0x0200 /* Advertise 1000 Mbps Full Duplex */
+
+#define IGB_GPIE_EIAME 0x10 /* Extended Interrupt Auto Mask Enable */
+#define IGB_IVAR_VALID 0x80 /* Valid bit for IVAR register */
+
+#define IGB_TXD_CMD_EOP 0x01 /* End of Packet */
+#define IGB_TXD_CMD_IFCS 0x02 /* Insert FCS */
+#define IGB_TXD_CMD_RS 0x08 /* Report Status */
+#define IGB_TXD_CMD_SHIFT 24 /* Shift for command bits in cmd_type_len */
+#define IGB_TXD_CMD_LEGACY_FORMAT (1 << 20) /* Forces legacy descriptor format in QEMU */
+
+#endif /* _IGB_REGISTERS_H_ */
diff --git a/tools/testing/selftests/vfio/lib/libvfio.mk b/tools/testing/selftests/vfio/lib/libvfio.mk
index 9f47bceed16f..1f13cca04348 100644
--- a/tools/testing/selftests/vfio/lib/libvfio.mk
+++ b/tools/testing/selftests/vfio/lib/libvfio.mk
@@ -12,6 +12,7 @@ LIBVFIO_C += vfio_pci_driver.c
 ifeq ($(ARCH:x86_64=x86),x86)
 LIBVFIO_C += drivers/ioat/ioat.c
 LIBVFIO_C += drivers/dsa/dsa.c
+LIBVFIO_C += drivers/igb/igb.c
 endif
 
 LIBVFIO_OUTPUT := $(OUTPUT)/libvfio
diff --git a/tools/testing/selftests/vfio/lib/vfio_pci_driver.c b/tools/testing/selftests/vfio/lib/vfio_pci_driver.c
index 6827f4a6febe..a5d0547132c4 100644
--- a/tools/testing/selftests/vfio/lib/vfio_pci_driver.c
+++ b/tools/testing/selftests/vfio/lib/vfio_pci_driver.c
@@ -5,12 +5,14 @@
 #ifdef __x86_64__
 extern struct vfio_pci_driver_ops dsa_ops;
 extern struct vfio_pci_driver_ops ioat_ops;
+extern struct vfio_pci_driver_ops igb_ops;
 #endif
 
 static struct vfio_pci_driver_ops *driver_ops[] = {
 #ifdef __x86_64__
 	&dsa_ops,
 	&ioat_ops,
+	&igb_ops,
 #endif
 };
 
-- 
2.54.0.794.g4f17f83d09-goog

Re: [PATCH v2 01/10] selftests/vfio: igb: Add driver for IGB QEMU device

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 12 potential issue(s) to consider:
- [High] Silent failures during device initialization leave the driver in an invalid state.
- [High] PCI_COMMAND_MEMORY enablement is skipped if PCI_COMMAND_MASTER is already set.
- [High] Device reset is asynchronous but not awaited, and MSI-X is disabled before the device is quiesced.
- [High] `igb_memcpy_start` fails to increment `src` and `dst` pointers for multiple chunks.
- [Medium] Missing SRRCTL configuration causes incorrect Rx descriptor write-back offset on real hardware.
- [Medium] Missing Transmit FCS (Frame Check Sequence) insertion causes payload truncation.
- [Medium] Lockless polling of DMA memory without READ_ONCE() or a volatile cast.
- [Medium] Incorrect MDI Error bit offset in MDIC register definition.
- [Medium] Missing read/write memory barriers around DMA descriptor and doorbell accesses.
- [Medium] Missing endianness conversion macros when populating hardware descriptors.
- [Medium] Unsafe hardware reset logic uses a sleep instead of polling the reset completion bit.
- [Medium] Incorrect bit definition for IGB_GPIE_EIAME silently disables auto-masking functionality.
--

commit f2aa2cf5b25f70be67a7ce0a012a1bdb1dba535c
Author: Josh Hilke <jrhilke@google.com>

selftests/vfio: igb: Add driver for IGB QEMU device
    
Add a VFIO selftest driver for the Intel Gigabit Ethernet controller
(IGB). IGB is fully virtualized in QEMU [1] which makes it easy to run
VFIO selftests without needing any specific hardware.

[1] https://www.qemu.org/docs/master/system/devices/igb.html
[2] https://github.com/arighi/virtme-ng

> diff --git a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
> new file mode 100644
> index 0000000000000..064ca7c05da02
> --- /dev/null
> +++ b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
[ ... ]
> +static void igb_init(struct vfio_pci_device *device)
> +{
[ ... ]
> +	/* Reset device and disable all interrupts */
> +	igb_write32(igb, IGB_CTRL, igb_read32(igb, IGB_CTRL) | IGB_CTRL_RST);
> +	usleep(20000);

[Severity: Medium]
Could this unsafe hardware reset logic cause issues by sleeping instead of
polling the reset completion bit? Accessing device registers while a reset is
propagating internally could trigger PCIe bus hangs.

> +	igb_write32(igb, IGB_IMC, 0xFFFFFFFF);
> +
> +	/* Signal that the driver is loaded */
> +	ctrl = igb_read32(igb, IGB_CTRL_EXT);
> +	ctrl |= IGB_CTRL_EXT_DRV_LOAD;
> +	ctrl &= ~IGB_CTRL_EXT_LINK_MODE_MASK;
> +	igb_write32(igb, IGB_CTRL_EXT, ctrl);
> +
> +	/* Enable PCI Bus Master. */
> +	cmd_reg = vfio_pci_config_readw(device, PCI_COMMAND);
> +	if (!(cmd_reg & PCI_COMMAND_MASTER)) {
> +		cmd_reg |= (PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY);
> +		vfio_pci_config_writew(device, PCI_COMMAND, cmd_reg);
> +	}

[Severity: High]
Does this logic inadvertently skip enabling PCI_COMMAND_MEMORY if
PCI_COMMAND_MASTER is already set? If the Bus Master bit is already enabled
by firmware or VFIO but Memory Space is disabled, the block is skipped, which
would leave Memory Space disabled and cause subsequent MMIO accesses to
trigger bus faults.

> +
> +	/* Trigger autonegotiation. This enables IGB to transmit data. */
> +	if (igb_phy_setup_autoneg(igb))
> +		return;

[Severity: High]
Does this early return result in a silent failure during device initialization?
If it fails, crucial setup steps are skipped, leaving variables like
max_memcpy_size uninitialized. The test framework will proceed to use the
partially configured device, leading to undefined behavior or crashes.
Could it explicitly assert failure via VFIO_ASSERT() instead?

> +
> +	/* Configure TX and RX descriptor rings */
> +	igb_write32(igb, IGB_TDBAL0, (u32)iova_tx);
> +	igb_write32(igb, IGB_TDBAH0, (u32)(iova_tx >> 32));
> +	igb_write32(igb, IGB_TDLEN0, RING_SIZE * sizeof(struct igb_tx_desc));
> +	igb_write32(igb, IGB_TDH0, 0);
> +	igb_write32(igb, IGB_TDT0, 0);
> +	igb_write32(igb, IGB_TXDCTL0, IGB_TXDCTL0_Q_EN);
> +
> +	igb_write32(igb, IGB_RDBAL0, (u32)iova_rx);

[Severity: Medium]
Is the SRRCTL configuration missing here? The driver expects the hardware to
use Advanced Receive Descriptors and polls for the DD bit at byte offset 8,
but fails to configure the SRRCTL register. Real hardware defaults to Legacy
Receive Descriptors (DD bit at offset 4), causing polling timeouts.

I noticed this is fixed in a later commit in the series: "selftests/vfio: igb:
Use advanced TX and RX descriptors".

[ ... ]
> +static void igb_remove(struct vfio_pci_device *device)
> +{
> +	struct igb *igb = to_igb_state(device);
> +
> +	vfio_pci_msix_disable(device);
> +	igb_write32(igb, IGB_RCTL, 0);
> +	igb_write32(igb, IGB_TCTL, 0);
> +	igb_write32(igb, IGB_CTRL, igb_read32(igb, IGB_CTRL) | IGB_CTRL_RST);
> +}

[Severity: High]
Is it safe to disable MSI-X before the device is quiesced, and trigger an
asynchronous device reset without awaiting its completion?

Calling vfio_pci_msix_disable() before masking interrupts and triggering a
hardware reset risks rogue interrupts during teardown. Returning immediately
after setting IGB_CTRL_RST might create a race condition where the VFIO
framework destroys IOMMU mappings while the device is still resetting,
potentially leading to a DMA use-after-free.

[ ... ]
> +static void igb_memcpy_start(struct vfio_pci_device *device, iova_t src,
> +			     iova_t dst, u64 size, u64 count)
> +{
> +	struct igb *igb = to_igb_state(device);
> +	struct igb_rx_desc *rx;
> +	struct igb_tx_desc *tx;
> +	u32 i;
> +
> +	igb_irq_disable(igb);
> +
> +	for (i = 0; i < count; i++) {
> +		tx = &igb->tx_ring[igb->tx_tail];
> +		rx = &igb->rx_ring[igb->rx_tail];
> +
> +		memset(tx, 0, sizeof(struct igb_tx_desc));
> +		memset(rx, 0, sizeof(struct igb_rx_desc));
> +
> +		rx->read.pkt_addr = dst;
> +		tx->read.buffer_addr = src;
> +		tx->read.cmd_type_len = (u32)size;
> +		tx->read.cmd_type_len |= (u32)(IGB_TXD_CMD_EOP) << IGB_TXD_CMD_SHIFT;

[Severity: High]
Does this loop fail to increment the src and dst pointers for multiple chunks?
When count > 1, the device repeatedly copies the first chunk of data to the
exact same destination address, leaving the remainder of the buffer uncopied
and causing data verification failures.

[Severity: Medium]
Are endianness conversion macros missing here? Populating 64-bit addresses
and 32-bit fields in hardware descriptors using native CPU byte order instead
of cpu_to_le32 and cpu_to_le64 will break on big-endian host architectures.

[Severity: Medium]
Should the Transmit FCS (Frame Check Sequence) insertion flag be present?
The driver sets IGB_RCTL_SECRC to strip Ethernet CRC on RX but omits the
IGB_TXD_CMD_IFCS flag on TX. On real hardware, no CRC is appended, causing
the RX MAC to strip the last 4 bytes of the actual payload.

I noticed this is fixed in a later commit in the series: "selftests/vfio: igb:
Use advanced TX and RX descriptors".

> +
> +		/* Set to 0 to disable offloads and avoid needing a context descriptor */
> +		tx->read.olinfo_status = 0;
> +
> +		igb->tx_tail = (igb->tx_tail + 1) % RING_SIZE;
> +		igb->rx_tail = (igb->rx_tail + 1) % RING_SIZE;
> +	}
> +
> +	igb_write32(igb, IGB_RDT0, igb->rx_tail);
> +	igb_write32(igb, IGB_TDT0, igb->tx_tail);

[Severity: Medium]
Are write memory barriers needed here around the DMA descriptor and doorbell
accesses? Without a write memory barrier between populating RAM descriptors
and ringing the TX doorbell, the hardware might fetch stale descriptors.

> +}
> +
> +static int igb_memcpy_wait(struct vfio_pci_device *device)
> +{
> +	struct igb *igb = to_igb_state(device);
> +	struct igb_rx_desc *rx;
> +	u32 prev_tail;
> +	int retries;
> +
> +	prev_tail = (igb->rx_tail + RING_SIZE - 1) % RING_SIZE;
> +	rx = &igb->rx_ring[prev_tail];
> +
> +	retries = 100;
> +	while (retries-- > 0) {
> +		if (rx->wb.status_error & 1)

[Severity: Medium]
Could this lockless polling of DMA memory without READ_ONCE() or a volatile
cast cause problems? The loop polls rx->wb.status_error, which allows the
compiler to theoretically cache the value in a register. While the external
usleep() call typically acts as an implicit compiler barrier, relying on it
is brittle and constitutes a data race.

> +			break;
> +		usleep(10);
> +	}
> +
> +	igb_irq_clear(igb);
> +
> +	igb_irq_enable(igb);
> +
> +	if (rx->wb.status_error & 1)

[Severity: Medium]
Is a read memory barrier missing here after polling the DMA completion status?
Without it, there could be speculative CPU reads of the destination buffer.

> +		return 0;
> +
> +	return -ETIMEDOUT;
> +}
[ ... ]
> diff --git a/tools/testing/selftests/vfio/lib/drivers/igb/registers.h b/tools/testing/selftests/vfio/lib/drivers/igb/registers.h
> new file mode 100644
> index 0000000000000..e2756be2e9c0c
> --- /dev/null
> +++ b/tools/testing/selftests/vfio/lib/drivers/igb/registers.h
[ ... ]
> +#define IGB_MDIC_PHY_SHIFT 21 /* PHY Address Shift */
> +#define IGB_MDIC_REG_SHIFT 16 /* Register Address Shift */
> +#define IGB_MDIC_READY (1 << 28) /* MDI Data Ready */
> +#define IGB_MDIC_ERROR (1 << 29) /* MDI Error */

[Severity: Medium]
Is this MDI Error bit offset correct in the MDIC register definition?
The macro is defined as (1 << 29), but according to the datasheet, bit 29 is
the Interrupt Enable bit, while the actual MDI Error bit is bit 30.

I noticed this is fixed in a later commit in the series: "selftests/vfio: igb:
Use offical IGB headers in selftest driver".

[ ... ]
> +#define IGB_CR_1000T_FD_CAPS 0x0200 /* Advertise 1000 Mbps Full Duplex */
> +
> +#define IGB_GPIE_EIAME 0x10 /* Extended Interrupt Auto Mask Enable */

[Severity: Medium]
Does this incorrect bit definition for IGB_GPIE_EIAME silently disable
auto-masking functionality? It is defined as 0x10, which corresponds to
Multiple_MSIX. The actual Extended Interrupt Auto Mask Enable bit is bit 30.
Writing 0x10 fails to enable auto-masking.

I noticed this is fixed in a later commit in the series: "selftests/vfio: igb:
Program MSI-X interrupt routing".

> +#define IGB_IVAR_VALID 0x80 /* Valid bit for IVAR register */
> +
> +#define IGB_TXD_CMD_EOP 0x01 /* End of Packet */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260526235417.2058313-1-jrhilke@google.com?part=1

[PATCH v2 02/10] selftests/vfio: igb: Use PHY internal loopback on 82576

[Josh Hilke]

From: Alex Williamson <alex.williamson@nvidia.com>

The submitted driver waits for PHY autonegotiation and then enables MAC
loopback via RCTL.LBM_MAC.  QEMU's emulated igb tolerates this, but the
82576 datasheet rejects the MAC loopback path on real hardware:

  Section 3.5.6.1 (Loopback Support / General): "Use PHY Loopback
  instead of MAC Loopback on the 82576."

  Section 3.5.6.2 (MAC Loopback): "MAC Loopback is not used on this
  device."

  Section 3.5.6.3.1 (Setting the 82576 to PHY loopback Mode): set PHY
  control register bits 8 (duplex), 14 (loopback), clear bit 12
  (autoneg enable), set the speed via bits 6 and 13.  For 1Gb/s the
  register value is 0x4140.

  Section 8.10.1 (RCTL register): "When using the internal PHY, LBM
  should remain set to 00b and the PHY instead configured for
  loopback through the MDIO interface."

Replace igb_phy_setup_autoneg() with igb_setup_loopback() which:

  - writes PHY register 0 with LOOPBACK | SPEED_1000 | FULL_DUPLEX
  - forces the MAC into 1Gb/s full duplex via CTRL.FRCSPD, CTRL.FRCDPX,
    CTRL.SPD_1000, CTRL.FD, CTRL.SLU; without forcing the MAC link
    state, the descriptor engine does not run on real hardware

PHY internal loopback (section 3.5.6.3) wraps data at the end of the
PHY datapath before the MDI, so the physical link state and cable
speed are irrelevant.  This matches the kernel ethtool selftest path
in igb_integrated_phy_loopback()
(drivers/net/ethernet/intel/igb/igb_ethtool.c).

QEMU's igb emulation drives STATUS.LU exclusively from its autoneg-done
timer or a network-backend link-state change; the guest cannot set
STATUS.LU through CTRL.SLU.  Its receive path checks STATUS.LU
(e1000x_hw_rx_enabled in hw/net/e1000x_common.c) and drops every
loopback frame until LU is set.  Issue a one-shot autoneg-restart PHY
write at the top of igb_setup_loopback() to kick the timer; the
subsequent PHY write clears autoneg-enable, so on real hardware
autoneg never starts and the write is a no-op.

QEMU's igb also does not honor PHY register 0 bit 14 (PHY internal
loopback) and relies on RCTL.LBM_MAC to wrap TX descriptors back to
the RX queue.  Datasheet 8.10.1 advises that LBM remain 00b when
using the internal PHY, but empirically setting LBM_MAC has no
observable effect on real 82576 (MAC loopback is not implemented per
3.5.6.2), so set it alongside PHY loopback as the actual loopback
mechanism under QEMU.  With these two QEMU-only accommodations the
selftest works in both environments without environment-specific code
paths.

The submitted driver also wrote bit 14 (link disable) to PHY register
16 (port control).  Datasheet 3.5.6.3.1 describes this as required for
10/100Mb/s but explicitly "not a must for 1G", and the kernel ethtool
selftest omits it.  It is dropped here.  Flagging the removal in case a
future regression bisects to this commit.

Remove igb_read_phy() and the PHY status macros it served, which become
unused: IGB_PHY_STATUS_REG_OFFSET, IGB_PHY_STATUS_AN_COMP.  Keep
IGB_PHY_CTRL_AN_ENABLE and IGB_PHY_CTRL_AN_RESTART for the QEMU-only
autoneg kick; add the PHY ctrl loopback and CTRL force-link macros.

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Alex Williamson <alex.williamson@nvidia.com>
---
 .../selftests/vfio/lib/drivers/igb/igb.c      | 126 ++++++++++--------
 .../vfio/lib/drivers/igb/registers.h          |  26 ++--
 2 files changed, 89 insertions(+), 63 deletions(-)

diff --git a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
index 064ca7c05da0..5bbc6fca7518 100644
--- a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
+++ b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
@@ -98,61 +98,71 @@ static int igb_write_phy(struct igb *igb, u32 offset, u16 data)
 	return 0;
 }
 
-static int igb_read_phy(struct igb *igb, u32 offset, u16 *data)
+/*
+ * Configure the device for PHY internal loopback per 82576 datasheet
+ * section 3.5.6.3.1.  Force the PHY to 1Gb/s full duplex with loopback
+ * enabled, then force the MAC link state to match.  Internal loopback
+ * wraps data at the end of the PHY datapath (section 3.5.6.3), so the
+ * physical link state is irrelevant.
+ *
+ * Section 3.5.6.1 directs to "Use PHY Loopback instead of MAC Loopback
+ * on the 82576", and section 3.5.6.2 states "MAC Loopback is not used
+ * on this device."  RCTL.LBM_MAC is still set elsewhere as a QEMU-only
+ * accommodation; see the RCTL programming in the caller for the
+ * rationale.
+ */
+static int igb_setup_loopback(struct igb *igb)
 {
-	u32 mdic;
-	int i;
-
-	mdic = ((offset << IGB_MDIC_REG_SHIFT) |
-		(1 << IGB_MDIC_PHY_SHIFT) |
-		IGB_MDIC_OP_READ);
-
-	igb_write32(igb, IGB_MDIC, mdic);
-
-	for (i = 0; i < 1000; i++) {
-		usleep(50);
-		mdic = igb_read32(igb, IGB_MDIC);
-		if (mdic & IGB_MDIC_READY)
-			break;
-	}
-
-	if (!(mdic & IGB_MDIC_READY))
-		return -1;
-
-	if (mdic & IGB_MDIC_ERROR)
-		return -1;
-
-	*data = (u16)mdic;
-	return 0;
-}
-
-static int igb_phy_setup_autoneg(struct igb *igb)
-{
-	int timeout_ms = 1000;
-	bool success = false;
-	u16 phy_status;
+	u32 ctrl;
 	int ret;
-	int i;
 
-	/* Trigger auto-negotiation */
+	/*
+	 * Kick the autoneg machinery solely to bring STATUS.LU up under
+	 * QEMU's igb emulation: QEMU only updates STATUS.LU via its
+	 * autoneg-done timer, and without LU set its receive path
+	 * (e1000x_hw_rx_enabled) drops every loopback frame.  On real
+	 * hardware autoneg cannot complete before the next PHY write
+	 * below clears the autoneg-enable bit, so this is effectively a
+	 * no-op there.
+	 */
+	(void)igb_write_phy(igb, IGB_PHY_CTRL_REG_OFFSET,
+			    IGB_PHY_CTRL_AN_ENABLE | IGB_PHY_CTRL_AN_RESTART);
+
+	/* PHY control: loopback + 1Gb/s full duplex, autoneg disabled. */
 	ret = igb_write_phy(igb, IGB_PHY_CTRL_REG_OFFSET,
-			    IGB_PHY_CTRL_AN_ENABLE | IGB_PHY_CTRL_RESTART_AN);
+			    IGB_PHY_CTRL_LOOPBACK |
+			    IGB_PHY_CTRL_SPEED_1000 |
+			    IGB_PHY_CTRL_FULL_DUPLEX);
 	if (ret)
 		return ret;
 
-	for (i = 0; i < timeout_ms; i++) {
-		if (igb_read_phy(igb, IGB_PHY_STATUS_REG_OFFSET, &phy_status) == 0) {
-			success = !!(phy_status & IGB_PHY_STATUS_AN_COMP);
-			if (success)
-				break;
-		}
-		usleep(1000);
-	}
-
-	if (!success) {
-		printf("igb: Auto-negotiation did not complete in time\n");
-		return -ETIMEDOUT;
-	}
+	/*
+	 * Brief delay before forcing the MAC, mirroring the kernel ethtool
+	 * selftest in igb_integrated_phy_loopback().  Not specified by the
+	 * datasheet, but empirically required by the kernel driver.
+	 */
+	usleep(50000);
+
+	/*
+	 * Force the MAC to 1Gb/s full duplex with link up.  Without forcing
+	 * the link state the descriptor engine does not run, since the chip
+	 * normally waits for a real negotiated link.
+	 */
+	ctrl = igb_read32(igb, IGB_CTRL);
+	ctrl &= ~IGB_CTRL_SPD_SEL;
+	ctrl |= IGB_CTRL_FRCSPD |
+		IGB_CTRL_FRCDPX |
+		IGB_CTRL_SPD_1000 |
+		IGB_CTRL_FD |
+		IGB_CTRL_SLU;
+	igb_write32(igb, IGB_CTRL, ctrl);
+
+	/*
+	 * Settling delay matching the kernel ethtool selftest's msleep(500)
+	 * at the tail of igb_integrated_phy_loopback().  Not specified by
+	 * the datasheet; empirical, and inherited from the kernel driver.
+	 */
+	usleep(500000);
 
 	return 0;
 }
@@ -199,8 +209,8 @@ static void igb_init(struct vfio_pci_device *device)
 		vfio_pci_config_writew(device, PCI_COMMAND, cmd_reg);
 	}
 
-	/* Trigger autonegotiation. This enables IGB to transmit data. */
-	if (igb_phy_setup_autoneg(igb))
+	/* Configure PHY internal loopback for testing. */
+	if (igb_setup_loopback(igb))
 		return;
 
 	/* Configure TX and RX descriptor rings */
@@ -227,10 +237,22 @@ static void igb_init(struct vfio_pci_device *device)
 		usleep(10);
 	}
 
-	/* Enable Receiver and Transmitter */
+	/*
+	 * Enable Receiver and Transmitter.  RCTL.LBM_MAC is set in addition
+	 * to PHY loopback as a QEMU-only accommodation: QEMU's emulated igb
+	 * does not honor PHY register 0 bit 14 (PHY internal loopback) and
+	 * relies on RCTL.LBM_MAC to wrap TX descriptors back to the RX
+	 * queue.  Datasheet 8.10.1 (RCTL register) advises "When using the
+	 * internal PHY, LBM should remain set to 00b", so setting LBM_MAC
+	 * here deviates from datasheet guidance; empirically the bit has
+	 * no observable effect on real 82576 hardware because MAC loopback
+	 * is not implemented (datasheet 3.5.6.2).  Setting both lets the
+	 * selftest work on both real hardware and QEMU without conditional
+	 * code paths.
+	 */
 	rctl = IGB_RCTL_EN |       /* Receiver Enable */
 	       IGB_RCTL_UPE |      /* Unicast Promiscuous (for dummy MAC) */
-	       IGB_RCTL_LBM_MAC |  /* MAC Loopback Mode */
+	       IGB_RCTL_LBM_MAC |  /* MAC Loopback - for QEMU emulation only */
 	       IGB_RCTL_SECRC;     /* Strip CRC (needed for memcmp) */
 	igb_write32(igb, IGB_RCTL, rctl);
 	igb_write32(igb, IGB_TCTL, IGB_TCTL_EN);
diff --git a/tools/testing/selftests/vfio/lib/drivers/igb/registers.h b/tools/testing/selftests/vfio/lib/drivers/igb/registers.h
index e2756be2e9c0..388862d481eb 100644
--- a/tools/testing/selftests/vfio/lib/drivers/igb/registers.h
+++ b/tools/testing/selftests/vfio/lib/drivers/igb/registers.h
@@ -34,7 +34,13 @@
 
 /* Control Bit Definitions */
 /* CTRL */
-#define IGB_CTRL_RST (1 << 26) /* Device Reset */
+#define IGB_CTRL_FD		(1 << 0)  /* Full Duplex */
+#define IGB_CTRL_SLU		(1 << 6)  /* Set Link Up */
+#define IGB_CTRL_SPD_SEL	(3 << 8) /* Speed Select Mask */
+#define IGB_CTRL_SPD_1000	(2 << 8) /* Force 1000 Mb/s */
+#define IGB_CTRL_FRCSPD		(1 << 11) /* Force Speed */
+#define IGB_CTRL_FRCDPX		(1 << 12) /* Force Duplex */
+#define IGB_CTRL_RST		(1 << 26) /* Device Reset */
 #define IGB_CTRL_EXT_LINK_MODE_MASK (3 << 22)
 
 /* CTRL_EXT */
@@ -44,7 +50,7 @@
 #define IGB_RCTL_EN (1 << 1) /* Receiver Enable */
 #define IGB_RCTL_UPE (1 << 3) /* Unicast Promiscuous Enabled */
 #define IGB_RCTL_LPE (1 << 5) /* Long Packet Reception Enable */
-#define IGB_RCTL_LBM_MAC (1 << 6) /* Loopback Mode - MAC */
+#define IGB_RCTL_LBM_MAC (1 << 6) /* Loopback Mode - MAC (set as QEMU-only accommodation) */
 #define IGB_RCTL_SECRC (1 << 26) /* Strip Ethernet CRC */
 
 /* TCTL */
@@ -81,15 +87,13 @@
 #define IGB_MDIC_READY (1 << 28) /* MDI Data Ready */
 #define IGB_MDIC_ERROR (1 << 29) /* MDI Error */
 
-#define IGB_PHY_CTRL_REG_OFFSET 0
-#define IGB_PHY_STATUS_REG_OFFSET 1
-#define IGB_PHY_STATUS_AN_COMP 0x0020 /* Auto-negotiation complete */
-#define IGB_PHY_CTRL_AN_ENABLE 0x1000 /* Auto-Negotiation Enable */
-#define IGB_PHY_CTRL_RESTART_AN 0x0200 /* Restart Auto-Negotiation */
-
-#define IGB_PHY_AUTONEG_ADV_REG_OFFSET 4 /* Auto-Negotiation Advertisement */
-#define IGB_PHY_1000T_CTRL_REG_OFFSET 9 /* 1000Base-T Control */
-#define IGB_CR_1000T_FD_CAPS 0x0200 /* Advertise 1000 Mbps Full Duplex */
+/* PHY register 0 (Control), per 82576 datasheet section 3.5.6.3.1 */
+#define IGB_PHY_CTRL_REG_OFFSET		0
+#define IGB_PHY_CTRL_AN_RESTART		0x0200 /* bit 9 */
+#define IGB_PHY_CTRL_AN_ENABLE		0x1000 /* bit 12 */
+#define IGB_PHY_CTRL_SPEED_1000		0x0040 /* bit 6 set, bit 13 clear */
+#define IGB_PHY_CTRL_FULL_DUPLEX	0x0100 /* bit 8 */
+#define IGB_PHY_CTRL_LOOPBACK		0x4000 /* bit 14 */
 
 #define IGB_GPIE_EIAME 0x10 /* Extended Interrupt Auto Mask Enable */
 #define IGB_IVAR_VALID 0x80 /* Valid bit for IVAR register */
-- 
2.54.0.794.g4f17f83d09-goog

[PATCH v2 03/10] selftests/vfio: igb: Use advanced TX and RX descriptors

[Josh Hilke]

From: Alex Williamson <alex.williamson@nvidia.com>

The submitted driver builds a partial legacy TX descriptor (just
DTALEN | CMD_EOP) and never programs SRRCTL.DESCTYPE.  QEMU's emulated
igb tolerates this by treating descriptors as advanced regardless of
DESCTYPE, but real 82576 hardware does not.

For receive, 82576 datasheet section 7.1.5.2 states: "SRRCTL[n].DESCTYPE
must be set to a value other than 000b for the 82576 to write back the
special descriptors."  struct igb_rx_desc matches the advanced
one-buffer writeback layout, so the test polls rx.wb.status_error,
which is only written in that layout.  Section 8.10.2 places DESCTYPE
in SRRCTL bits 27:25; program it with 001b (advanced one-buffer).

For transmit, datasheet section 7.2.2.3 describes the advanced data
descriptor with DEXT (DCMD bit 5) marking the descriptor as advanced,
DTYP=0011b selecting the data descriptor, IFCS (DCMD bit 1) asking the
MAC to append the Ethernet FCS (without it the frame is dropped as
malformed), EOP (DCMD bit 0) marking end of packet, and PAYLEN in
olinfo_status[31:14] carrying the total payload size.  Build this
descriptor in igb_memcpy_start().

Remove the legacy CMD macros (IGB_TXD_CMD_EOP, IGB_TXD_CMD_IFCS,
IGB_TXD_CMD_RS, IGB_TXD_CMD_SHIFT, IGB_TXD_CMD_LEGACY_FORMAT) that
become unused, and add the SRRCTL register offset, DESCTYPE encoding,
and advanced TX descriptor field macros.

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Alex Williamson <alex.williamson@nvidia.com>
---
 .../selftests/vfio/lib/drivers/igb/igb.c      | 38 ++++++++++++++++---
 .../vfio/lib/drivers/igb/registers.h          | 21 +++++++---
 2 files changed, 49 insertions(+), 10 deletions(-)

diff --git a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
index 5bbc6fca7518..c0f9cefb23e5 100644
--- a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
+++ b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
@@ -226,6 +226,17 @@ static void igb_init(struct vfio_pci_device *device)
 	igb_write32(igb, IGB_RDLEN0, RING_SIZE * sizeof(struct igb_rx_desc));
 	igb_write32(igb, IGB_RDH0, 0);
 	igb_write32(igb, IGB_RDT0, 0);
+
+	/*
+	 * Select the advanced one-buffer descriptor format.  Per 82576
+	 * datasheet section 7.1.5.2: "SRRCTL[n].DESCTYPE must be set to a
+	 * value other than 000b for the 82576 to write back the special
+	 * descriptors."  struct igb_rx_desc matches the advanced one-buffer
+	 * writeback layout (section 7.1.5.2), so polling rx.wb.status_error
+	 * requires this format.  Section 8.10.2 specifies DESCTYPE[27:25].
+	 */
+	igb_write32(igb, IGB_SRRCTL0, IGB_SRRCTL_DESCTYPE_ADV_ONEBUF);
+
 	igb_write32(igb, IGB_RXDCTL0, IGB_RXDCTL0_Q_EN);
 
 	/* Wait for TX and RX queues to be enabled */
@@ -322,12 +333,29 @@ static void igb_memcpy_start(struct vfio_pci_device *device, iova_t src,
 		memset(rx, 0, sizeof(struct igb_rx_desc));
 
 		rx->read.pkt_addr = dst;
-		tx->read.buffer_addr = src;
-		tx->read.cmd_type_len = (u32)size;
-		tx->read.cmd_type_len |= (u32)(IGB_TXD_CMD_EOP) << IGB_TXD_CMD_SHIFT;
+		rx->read.hdr_addr = 0;
 
-		/* Set to 0 to disable offloads and avoid needing a context descriptor */
-		tx->read.olinfo_status = 0;
+		tx->read.buffer_addr = src;
+		/*
+		 * Build an advanced data descriptor per 82576 datasheet
+		 * section 7.2.2.3.  DEXT marks the descriptor as advanced
+		 * (required by hardware); DTYP=data selects the data
+		 * descriptor; IFCS asks the MAC to append the Ethernet
+		 * FCS (without it the frame is dropped as malformed);
+		 * EOP marks end of packet.  DTALEN is the buffer length
+		 * in bits 15:0 of cmd_type_len.
+		 */
+		tx->read.cmd_type_len = (uint32_t)size |
+			IGB_ADVTXD_DTYP_DATA |
+			IGB_ADVTXD_DCMD_DEXT |
+			IGB_ADVTXD_DCMD_IFCS |
+			IGB_ADVTXD_DCMD_EOP;
+		/*
+		 * PAYLEN (section 7.2.2.3.11) is the total payload size
+		 * in olinfo_status[31:14].
+		 */
+		tx->read.olinfo_status =
+			(uint32_t)size << IGB_ADVTXD_PAYLEN_SHIFT;
 
 		igb->tx_tail = (igb->tx_tail + 1) % RING_SIZE;
 		igb->rx_tail = (igb->rx_tail + 1) % RING_SIZE;
diff --git a/tools/testing/selftests/vfio/lib/drivers/igb/registers.h b/tools/testing/selftests/vfio/lib/drivers/igb/registers.h
index 388862d481eb..9a8b0f830f89 100644
--- a/tools/testing/selftests/vfio/lib/drivers/igb/registers.h
+++ b/tools/testing/selftests/vfio/lib/drivers/igb/registers.h
@@ -20,10 +20,14 @@
 #define IGB_RDBAL0 0x0C000 /* Rx Desc Base Address Low */
 #define IGB_RDBAH0 0x0C004 /* Rx Desc Base Address High */
 #define IGB_RDLEN0 0x0C008 /* Rx Desc Length */
+#define IGB_SRRCTL0 0x0C00C /* Split and Replication Receive Control Q0 */
 #define IGB_RDH0 0x0C010 /* Rx Desc Head */
 #define IGB_RDT0 0x0C018 /* Rx Desc Tail */
 #define IGB_RXDCTL0 0x0C028 /* Rx Desc Control */
 
+/* SRRCTL fields per 82576 datasheet section 8.10.2 */
+#define IGB_SRRCTL_DESCTYPE_ADV_ONEBUF (1u << 25) /* 001b: advanced one-buffer */
+
 /* Tx Ring 0 Registers */
 #define IGB_TDBAL0 0x0E000 /* Tx Desc Base Address Low */
 #define IGB_TDBAH0 0x0E004 /* Tx Desc Base Address High */
@@ -98,10 +102,17 @@
 #define IGB_GPIE_EIAME 0x10 /* Extended Interrupt Auto Mask Enable */
 #define IGB_IVAR_VALID 0x80 /* Valid bit for IVAR register */
 
-#define IGB_TXD_CMD_EOP 0x01 /* End of Packet */
-#define IGB_TXD_CMD_IFCS 0x02 /* Insert FCS */
-#define IGB_TXD_CMD_RS 0x08 /* Report Status */
-#define IGB_TXD_CMD_SHIFT 24 /* Shift for command bits in cmd_type_len */
-#define IGB_TXD_CMD_LEGACY_FORMAT (1 << 20) /* Forces legacy descriptor format in QEMU */
+/*
+ * Advanced TX Data Descriptor fields per 82576 datasheet section 7.2.2.3.
+ * The cmd_type_len word holds: DTALEN[15:0], MAC[19:18], DTYP[23:20],
+ * DCMD[31:24].  The olinfo_status word holds: STA[3:0], IDX[6:4],
+ * POPTS[13:8], PAYLEN[31:14].
+ */
+#define IGB_ADVTXD_DTYP_DATA	(0x3u << 20) /* DTYP=0011b: advanced data */
+#define IGB_ADVTXD_DCMD_EOP	(1u << 24)   /* DCMD bit 0: End of Packet */
+#define IGB_ADVTXD_DCMD_IFCS	(1u << 25)   /* DCMD bit 1: Insert FCS */
+#define IGB_ADVTXD_DCMD_RS	(1u << 27)   /* DCMD bit 3: Report Status */
+#define IGB_ADVTXD_DCMD_DEXT	(1u << 29)   /* DCMD bit 5: 1b for advanced */
+#define IGB_ADVTXD_PAYLEN_SHIFT	14           /* PAYLEN bit position */
 
 #endif /* _IGB_REGISTERS_H_ */
-- 
2.54.0.794.g4f17f83d09-goog

Re: [PATCH v2 03/10] selftests/vfio: igb: Use advanced TX and RX descriptors

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Overwriting IGB_SRRCTL0 clears the BSIZEPACKET field, setting the receive buffer size to 0 bytes and violating the hardware specification.
--

commit dc2cc67e8a6e2c641907cee5129b78515c56abca
Author: Alex Williamson <alex.williamson@nvidia.com>

selftests/vfio: igb: Use advanced TX and RX descriptors

This commit updates the igb selftests driver to use advanced descriptors for
transmit and receive. It configures SRRCTL.DESCTYPE to the advanced
one-buffer layout for receive and sets up advanced data descriptor fields
in igb_memcpy_start() for transmit. Legacy CMD macros are removed.

> diff --git a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
> index 5bbc6fca75189..c0f9cefb23e59 100644
> --- a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
> +++ b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
> @@ -226,6 +226,17 @@ static void igb_init(struct vfio_pci_device *device)
>  	igb_write32(igb, IGB_RDLEN0, RING_SIZE * sizeof(struct igb_rx_desc));
>  	igb_write32(igb, IGB_RDH0, 0);
>  	igb_write32(igb, IGB_RDT0, 0);
> +
> +	/*
> +	 * Select the advanced one-buffer descriptor format.  Per 82576
> +	 * datasheet section 7.1.5.2: "SRRCTL[n].DESCTYPE must be set to a
> +	 * value other than 000b for the 82576 to write back the special
> +	 * descriptors."  struct igb_rx_desc matches the advanced one-buffer
> +	 * writeback layout (section 7.1.5.2), so polling rx.wb.status_error
> +	 * requires this format.  Section 8.10.2 specifies DESCTYPE[27:25].
> +	 */
> +	igb_write32(igb, IGB_SRRCTL0, IGB_SRRCTL_DESCTYPE_ADV_ONEBUF);

[Severity: Medium]
Does writing directly to IGB_SRRCTL0 unintentionally clear the BSIZEPACKET
field?

Because this is a direct write rather than a read-modify-write, all other bits
in the register are cleared to 0, including BSIZEPACKET (bits 6:0).

The Intel 82576 datasheet dictates that BSIZEPACKET sets the receive buffer
size in 1 KB increments and explicitly forbids setting it to 0. A 0 value
leaves the hardware with no space configured for DMA, causing it to drop
incoming packets.

This behavior remains present in subsequent patches in the series, where
the macro is updated to E1000_SRRCTL(0) but the BSIZEPACKET field remains
unpopulated.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260526235417.2058313-1-jrhilke@google.com?part=3

Re: [PATCH v2 03/10] selftests/vfio: igb: Use advanced TX and RX descriptors

[Alex Williamson]

On Wed, 27 May 2026 00:14:44 +0000
sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] Overwriting IGB_SRRCTL0 clears the BSIZEPACKET field, setting the receive buffer size to 0 bytes and violating the hardware specification.
> --
> 
> commit dc2cc67e8a6e2c641907cee5129b78515c56abca
> Author: Alex Williamson <alex.williamson@nvidia.com>
> 
> selftests/vfio: igb: Use advanced TX and RX descriptors
> 
> This commit updates the igb selftests driver to use advanced descriptors for
> transmit and receive. It configures SRRCTL.DESCTYPE to the advanced
> one-buffer layout for receive and sets up advanced data descriptor fields
> in igb_memcpy_start() for transmit. Legacy CMD macros are removed.
> 
> > diff --git a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
> > index 5bbc6fca75189..c0f9cefb23e59 100644
> > --- a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
> > +++ b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
> > @@ -226,6 +226,17 @@ static void igb_init(struct vfio_pci_device *device)
> >  	igb_write32(igb, IGB_RDLEN0, RING_SIZE * sizeof(struct igb_rx_desc));
> >  	igb_write32(igb, IGB_RDH0, 0);
> >  	igb_write32(igb, IGB_RDT0, 0);
> > +
> > +	/*
> > +	 * Select the advanced one-buffer descriptor format.  Per 82576
> > +	 * datasheet section 7.1.5.2: "SRRCTL[n].DESCTYPE must be set to a
> > +	 * value other than 000b for the 82576 to write back the special
> > +	 * descriptors."  struct igb_rx_desc matches the advanced one-buffer
> > +	 * writeback layout (section 7.1.5.2), so polling rx.wb.status_error
> > +	 * requires this format.  Section 8.10.2 specifies DESCTYPE[27:25].
> > +	 */
> > +	igb_write32(igb, IGB_SRRCTL0, IGB_SRRCTL_DESCTYPE_ADV_ONEBUF);  
> 
> [Severity: Medium]
> Does writing directly to IGB_SRRCTL0 unintentionally clear the BSIZEPACKET
> field?
> 
> Because this is a direct write rather than a read-modify-write, all other bits
> in the register are cleared to 0, including BSIZEPACKET (bits 6:0).
> 
> The Intel 82576 datasheet dictates that BSIZEPACKET sets the receive buffer
> size in 1 KB increments and explicitly forbids setting it to 0. A 0 value
> leaves the hardware with no space configured for DMA, causing it to drop
> incoming packets.
> 
> This behavior remains present in subsequent patches in the series, where
> the macro is updated to E1000_SRRCTL(0) but the BSIZEPACKET field remains
> unpopulated.
> 

From the spec...

7.1.3.1 Host Buffers

  ...

  The size of the buffer can be set using either the generic RCTL.BSIZE
  field, or the per queue SRRCTL[n].BSIZEPACKET field.

  The receive buffer size is selected by bit settings in the Receive
  Control (RCTL.BSIZE). The register supports buffer sizes of 256, 512,
  1024, and 2048 bytes. See section 12.7.1 for details.

  If SRRCTL[n].BSIZEPACKET is set to zero for any queue, the buffer
  size defined by RCTL.BSIZE is used. Otherwise, the buffer size
  defined by SRRCTL[n].BSIZEPACKET is used.

8.10.1 Receive Control Register - RCTL (0x00100; R/W)

  Field   Bit(s)  Initial Value  Description ->

  BSIZE   17:16       00b

  Receive Buffer Size.
  BSIZE controls the size of the receive buffers and permits
  software to trade-off descriptor performance versus required
  storage space. Buffers that are 2048 bytes require only one
  descriptor per receive packet maximizing descriptor
  efficiency.

  00b = 2048 Bytes.
  01b = 1024 Bytes.
  10b = 512 Bytes.
  11b = 256 Bytes.

False positive.  Thanks,

Alex

[PATCH v2 04/10] selftests/vfio: igb: Program MSI-X interrupt routing

[Josh Hilke]

From: Alex Williamson <alex.williamson@nvidia.com>

The submitted driver writes only GPIE.EIAME (with a register value of
0x10, which is actually GPIE.Multiple_MSIX, bit 4) and clears EICR by
reading it.  On QEMU this works because the emulated loopback path is
synchronous and EICR is implemented as read-to-clear unconditionally.
Real 82576 hardware needs the full MSI-X programming sequence.

Per 82576 datasheet section 7.3.2.11 Table 7-47, MSI-X mode requires:

  GPIE.Multiple_MSIX (bit 4): route causes through IVAR.
  GPIE.EIAME (bit 30): apply EIAM on MSI-X assertion.  Without EIAME,
    section 7.3.2.11 specifies EIAM only takes effect on EICR
    read/write, which is not the path used here.

Configure auto-clear and auto-mask for vector 0:

  EIAC (section 8.8.5): auto-clear of EICR cause bit on MSI-X assertion.
  EIAM (section 8.8.6): with EIAME set, auto-mask of EIMS on MSI-X
    assertion.  This guarantees one interrupt per memcpy batch and
    prevents repeat delivery if the cause re-asserts before EIMS is
    restored.

Replace the read-to-clear of EICR with write-to-clear.  Section 8.8.5
states "If any bits are set in EIAC, the EICR register should not be
read", and section 7.3.4.3 cautions against read-to-clear in MSI-X
mode in general.  Write-to-clear (section 7.3.4.2) is unconditional.

Replace the magic '1' values written to EIMS/EIMC with IGB_EICR_VEC0,
add the GPIE/EIAC/EIAM macros, and drop the wrong-valued IGB_GPIE_EIAME
macro (the new definition lives next to IGB_GPIE_MULTIPLE_MSIX).

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Alex Williamson <alex.williamson@nvidia.com>
---
 .../selftests/vfio/lib/drivers/igb/igb.c      | 40 ++++++++++++++++---
 .../vfio/lib/drivers/igb/registers.h          | 11 ++++-
 2 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
index c0f9cefb23e5..a8f2072b6820 100644
--- a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
+++ b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
@@ -271,11 +271,32 @@ static void igb_init(struct vfio_pci_device *device)
 	/* Enable MSI-X with 1 vector for the test */
 	vfio_pci_msix_enable(device, MSIX_VECTOR, 1);
 
-	/* Enable auto-masking of interrupts to avoid storms without a real ISR */
-	igb_write32(igb, IGB_GPIE, IGB_GPIE_EIAME);
+	/*
+	 * Program MSI-X interrupt routing per 82576 datasheet:
+	 *
+	 * GPIE (section 7.3.2.11, Table 7-47): set Multiple_MSIX (bit 4) to
+	 * route interrupt causes through IVAR mapping, and EIAME (bit 30)
+	 * to apply EIAM on MSI-X assertion (without EIAME, EIAM only
+	 * applies on EICR read/write).
+	 *
+	 * EIAC (section 8.8.5): enable auto-clear of EICR for vector 0.
+	 * Without auto-clear the cause stays set after delivery and the
+	 * test can see spurious interrupts on the next memcpy batch.
+	 *
+	 * EIAM (section 8.8.6): enable auto-mask of EIMS for vector 0 on
+	 * MSI-X assertion (effective because EIAME is set), so a single
+	 * interrupt is delivered per memcpy batch even if the cause
+	 * re-asserts before software re-enables the mask.
+	 *
+	 * IVAR (section 7.3.1.2, register definition in 8.8.13): map RX
+	 * cause 0 to MSI-X vector 0 and mark the entry valid.
+	 */
+	igb_write32(igb, IGB_GPIE, IGB_GPIE_MULTIPLE_MSIX | IGB_GPIE_EIAME);
+	igb_write32(igb, IGB_EIAC, IGB_EICR_VEC0);
+	igb_write32(igb, IGB_EIAM, IGB_EICR_VEC0);
 
 	/* Enable interrupts on vector 0 */
-	igb_write32(igb, IGB_EIMS, 1);
+	igb_write32(igb, IGB_EIMS, IGB_EICR_VEC0);
 
 	/* Map vector 0 to interrupt cause 0 and mark it valid */
 	igb_write32(igb, IGB_IVAR0, IGB_IVAR_VALID);
@@ -302,17 +323,24 @@ static void igb_remove(struct vfio_pci_device *device)
 
 static void igb_irq_disable(struct igb *igb)
 {
-	igb_write32(igb, IGB_EIMC, 1);
+	igb_write32(igb, IGB_EIMC, IGB_EICR_VEC0);
 }
 
 static void igb_irq_enable(struct igb *igb)
 {
-	igb_write32(igb, IGB_EIMS, 1);
+	igb_write32(igb, IGB_EIMS, IGB_EICR_VEC0);
 }
 
 static void igb_irq_clear(struct igb *igb)
 {
-	igb_read32(igb, IGB_EICR);
+	/*
+	 * Use write-to-clear (datasheet 7.3.4.2).  In MSI-X mode with EIAC
+	 * programmed, section 8.8.5 explicitly states "If any bits are set
+	 * in EIAC, the EICR register should not be read", which rules out
+	 * the read-to-clear path in 7.3.4.3.  Bits not in EIAC are still
+	 * cleared by writing 1.
+	 */
+	igb_write32(igb, IGB_EICR, 0xFFFFFFFF);
 }
 
 static void igb_memcpy_start(struct vfio_pci_device *device, iova_t src,
diff --git a/tools/testing/selftests/vfio/lib/drivers/igb/registers.h b/tools/testing/selftests/vfio/lib/drivers/igb/registers.h
index 9a8b0f830f89..a133a208ed08 100644
--- a/tools/testing/selftests/vfio/lib/drivers/igb/registers.h
+++ b/tools/testing/selftests/vfio/lib/drivers/igb/registers.h
@@ -3,6 +3,8 @@
 #ifndef _IGB_REGISTERS_H_
 #define _IGB_REGISTERS_H_
 
+#include <linux/bits.h>
+
 /* Register Offsets (Intel 82576EB Datasheet) */
 #define IGB_CTRL 0x00000 /* Device Control */
 #define IGB_STATUS 0x00008 /* Device Status */
@@ -76,12 +78,18 @@
 #define IGB_VMOLR_BAM 0x08000000 /* Broadcast Accept Mode */
 #define IGB_RAH_POOL_1 0x00040000 /* Pool 1 assignment */
 
-#define IGB_EIMS 0x01524 /* Extended Interrupt Mask Set */
 #define IGB_EICS 0x01520 /* Extended Interrupt Cause Set */
+#define IGB_EIMS 0x01524 /* Extended Interrupt Mask Set */
 #define IGB_EIMC 0x01528 /* Extended Interrupt Mask Clear */
+#define IGB_EIAC 0x0152C /* Extended Interrupt Auto Clear */
+#define IGB_EIAM 0x01530 /* Extended Interrupt Auto Mask Enable */
+#define IGB_EICR_VEC0 BIT(0) /* MSI-X cause/vector 0 */
 #define IGB_CTRL_GIO_MASTER_DISABLE (1 << 2) /* GIO Master Disable */
 #define IGB_STATUS_GIO_MASTER_ENABLE (1 << 19) /* GIO Master Enable */
 #define IGB_GPIE 0x01514 /* General Purpose Interrupt Enable */
+/* GPIE fields per 82576 datasheet section 7.3.2.11, Table 7-47 */
+#define IGB_GPIE_MULTIPLE_MSIX BIT(4)  /* Multi-vector MSI-X mode */
+#define IGB_GPIE_EIAME         BIT(30) /* Apply EIAM on MSI-X assertion */
 #define IGB_TXDCTL0_Q_EN (1 << 25) /* Transmit Queue Enable */
 #define IGB_RXDCTL0_Q_EN (1 << 25) /* Receive Queue Enable */
 #define IGB_MRQC 0x05818 /* Multiple Receive Queues Command */
@@ -99,7 +107,6 @@
 #define IGB_PHY_CTRL_FULL_DUPLEX	0x0100 /* bit 8 */
 #define IGB_PHY_CTRL_LOOPBACK		0x4000 /* bit 14 */
 
-#define IGB_GPIE_EIAME 0x10 /* Extended Interrupt Auto Mask Enable */
 #define IGB_IVAR_VALID 0x80 /* Valid bit for IVAR register */
 
 /*
-- 
2.54.0.794.g4f17f83d09-goog

Re: [PATCH v2 04/10] selftests/vfio: igb: Program MSI-X interrupt routing

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] igb_send_msi() was overlooked by the patch, causing a state regression due to auto-masking and leaving a magic number unreplaced.
--

commit 059a9f7e058b59c775b2202ffd0af3e078efbd8a
Author: Alex Williamson <alex.williamson@nvidia.com>

selftests/vfio: igb: Program MSI-X interrupt routing

This commit programs the full MSI-X routing sequence for the igb driver in the
vfio selftests. It enables auto-clear and auto-mask for vector 0 and replaces
magic numbers with appropriate macros to match hardware behavior.

> diff --git a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
> index c0f9cefb23e59..a8f2072b68200 100644
> --- a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
> +++ b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
> @@ -271,11 +271,32 @@ static void igb_init(struct vfio_pci_device *device)
>  	/* Enable MSI-X with 1 vector for the test */
>  	vfio_pci_msix_enable(device, MSIX_VECTOR, 1);
>  
> -	/* Enable auto-masking of interrupts to avoid storms without a real ISR */
> -	igb_write32(igb, IGB_GPIE, IGB_GPIE_EIAME);
> +	/*
> +	 * Program MSI-X interrupt routing per 82576 datasheet:
> +	 *
> +	 * GPIE (section 7.3.2.11, Table 7-47): set Multiple_MSIX (bit 4) to
> +	 * route interrupt causes through IVAR mapping, and EIAME (bit 30)
> +	 * to apply EIAM on MSI-X assertion (without EIAME, EIAM only
> +	 * applies on EICR read/write).
> +	 *
> +	 * EIAC (section 8.8.5): enable auto-clear of EICR for vector 0.
> +	 * Without auto-clear the cause stays set after delivery and the
> +	 * test can see spurious interrupts on the next memcpy batch.
> +	 *
> +	 * EIAM (section 8.8.6): enable auto-mask of EIMS for vector 0 on
> +	 * MSI-X assertion (effective because EIAME is set), so a single
> +	 * interrupt is delivered per memcpy batch even if the cause
> +	 * re-asserts before software re-enables the mask.
> +	 *
> +	 * IVAR (section 7.3.1.2, register definition in 8.8.13): map RX
> +	 * cause 0 to MSI-X vector 0 and mark the entry valid.
> +	 */
> +	igb_write32(igb, IGB_GPIE, IGB_GPIE_MULTIPLE_MSIX | IGB_GPIE_EIAME);
> +	igb_write32(igb, IGB_EIAC, IGB_EICR_VEC0);
> +	igb_write32(igb, IGB_EIAM, IGB_EICR_VEC0);

[Severity: Medium]
Will this auto-masking configuration break software interrupt generation in
igb_send_msi()?

When igb_send_msi() writes to IGB_EICS to generate a software interrupt,
the hardware asserts the MSI-X message and auto-masks IGB_EIMS based on
this EIAM configuration.

Because igb_send_msi() does not explicitly unmask the interrupt before
triggering it, will consecutive calls to igb_send_msi() silently fail to
deliver the interrupt if there is no intervening unmask?

tools/testing/selftests/vfio/lib/drivers/igb/igb.c:igb_send_msi() {
...
	igb_write32(igb, IGB_EICS, 1);
...
}

Also, does the hardcoded '1' in igb_send_msi() need to be replaced with
IGB_EICR_VEC0 to match the other magic number replacements in this commit?

>  
>  	/* Enable interrupts on vector 0 */
> -	igb_write32(igb, IGB_EIMS, 1);
> +	igb_write32(igb, IGB_EIMS, IGB_EICR_VEC0);

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260526235417.2058313-1-jrhilke@google.com?part=4

Re: [PATCH v2 04/10] selftests/vfio: igb: Program MSI-X interrupt routing

[Alex Williamson]

On Wed, 27 May 2026 00:19:08 +0000
sashiko-bot@kernel.org wrote:

> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] igb_send_msi() was overlooked by the patch, causing a state regression due to auto-masking and leaving a magic number unreplaced.
> --
> 
> commit 059a9f7e058b59c775b2202ffd0af3e078efbd8a
> Author: Alex Williamson <alex.williamson@nvidia.com>
> 
> selftests/vfio: igb: Program MSI-X interrupt routing
> 
> This commit programs the full MSI-X routing sequence for the igb driver in the
> vfio selftests. It enables auto-clear and auto-mask for vector 0 and replaces
> magic numbers with appropriate macros to match hardware behavior.
> 
> > diff --git a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
> > index c0f9cefb23e59..a8f2072b68200 100644
> > --- a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
> > +++ b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
> > @@ -271,11 +271,32 @@ static void igb_init(struct vfio_pci_device *device)
> >  	/* Enable MSI-X with 1 vector for the test */
> >  	vfio_pci_msix_enable(device, MSIX_VECTOR, 1);
> >  
> > -	/* Enable auto-masking of interrupts to avoid storms without a real ISR */
> > -	igb_write32(igb, IGB_GPIE, IGB_GPIE_EIAME);
> > +	/*
> > +	 * Program MSI-X interrupt routing per 82576 datasheet:
> > +	 *
> > +	 * GPIE (section 7.3.2.11, Table 7-47): set Multiple_MSIX (bit 4) to
> > +	 * route interrupt causes through IVAR mapping, and EIAME (bit 30)
> > +	 * to apply EIAM on MSI-X assertion (without EIAME, EIAM only
> > +	 * applies on EICR read/write).
> > +	 *
> > +	 * EIAC (section 8.8.5): enable auto-clear of EICR for vector 0.
> > +	 * Without auto-clear the cause stays set after delivery and the
> > +	 * test can see spurious interrupts on the next memcpy batch.
> > +	 *
> > +	 * EIAM (section 8.8.6): enable auto-mask of EIMS for vector 0 on
> > +	 * MSI-X assertion (effective because EIAME is set), so a single
> > +	 * interrupt is delivered per memcpy batch even if the cause
> > +	 * re-asserts before software re-enables the mask.
> > +	 *
> > +	 * IVAR (section 7.3.1.2, register definition in 8.8.13): map RX
> > +	 * cause 0 to MSI-X vector 0 and mark the entry valid.
> > +	 */
> > +	igb_write32(igb, IGB_GPIE, IGB_GPIE_MULTIPLE_MSIX | IGB_GPIE_EIAME);
> > +	igb_write32(igb, IGB_EIAC, IGB_EICR_VEC0);
> > +	igb_write32(igb, IGB_EIAM, IGB_EICR_VEC0);  
> 
> [Severity: Medium]
> Will this auto-masking configuration break software interrupt generation in
> igb_send_msi()?
> 
> When igb_send_msi() writes to IGB_EICS to generate a software interrupt,
> the hardware asserts the MSI-X message and auto-masks IGB_EIMS based on
> this EIAM configuration.
> 
> Because igb_send_msi() does not explicitly unmask the interrupt before
> triggering it, will consecutive calls to igb_send_msi() silently fail to
> deliver the interrupt if there is no intervening unmask?

7.3.2.10  Extended Interrupt Auto Mask Enable Register (EIAM)

  Each bit set in this register enables clearing of the corresponding
  bit in the extended mask register following read or write-to-clear to
  EICR. It also enables setting of the corresponding bit in the
  extended mask register following a write-to-set to EICS.

Table 7-47

  EIAME: When set, upon firing of an MSI-X message, mask bits set in
  EIAM associated with this message are cleared. Otherwise, EIAM is
  used only upon read or write of EICR/EICS registers.

7.3.2.7  Extended Interrupt Cause Set Register (EICS)

  This registers allows to set the bits of EICR by software, by writing
  a 1b in the corresponding bits in EICS. Used usually to rearm
  interrupts the software didn't have time to handle in the current
  interrupt routine.

With EIAM[VEC0]=1 and EIMS[VEC0]=1 (set at init):

 1. igb_send_msi() writes EICS[VEC0]=1 → sets EICR[VEC0]

 2. Per 7.3.2.10, the write-to-set to EICS makes EIAM set EIMS[VEC0]m
    so the mask is armed by the very same operation, regardless of its
    prior state.

 3. EICR[VEC0]=1 && EIMS[VEC0]=1 → MSI-X message fires (per 7.3.2.8:
   "Interrupts appear on PCIe only if the interrupt cause bit is a one
   and the corresponding interrupt mask bit is a one").

 4. On assertion, with GPIE.EIAME=1, EIAM clears EIMS[VEC0] (one-shot throttle) and
   EIAC clears EICR[VEC0].

 5. Next igb_send_msi() → step 2 re-sets EIMS[VEC0] again. Self-arming.
    No external unmask required.

The auto-clear (step 4) that the sashiko focused on is exactly undone by
the auto-set on the next EICS write (step 2). The two halves of EIAM
are what make the software-MSI path self-contained.

Verified on hardware with back-to-back send_msi().  False positive.

> tools/testing/selftests/vfio/lib/drivers/igb/igb.c:igb_send_msi() {
> ...
> 	igb_write32(igb, IGB_EICS, 1);
> ...
> }
> 
> Also, does the hardcoded '1' in igb_send_msi() need to be replaced
>   with IGB_EICR_VEC0 to match the other magic number replacements in
>   this commit?

Yes, this should have included:

diff --git a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
index a8f2072b6820..b4f75965e18d 100644
--- a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
+++ b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
@@ -424,7 +424,7 @@ static void igb_send_msi(struct vfio_pci_device *device)
 {
 	struct igb *igb = to_igb_state(device);
 
-	igb_write32(igb, IGB_EICS, 1);
+	igb_write32(igb, IGB_EICS, IGB_EICR_VEC0);
 }
 
 const struct vfio_pci_driver_ops igb_ops = {

This should be added on commit or respin.  Thanks,

Alex

[PATCH v2 05/10] selftests/vfio: igb: Extend memcpy completion timeout for line-rate hardware

[Josh Hilke]

From: Alex Williamson <alex.williamson@nvidia.com>

The submitted driver waits at most 1 ms (100 * 10 us) for the last RX
descriptor to be written back.  QEMU's emulated loopback is synchronous:
by the time igb_memcpy_wait() runs, the receive descriptors are already
written back.  Real 82576 hardware processes the descriptor ring at
line rate.

max_memcpy_size is (RING_SIZE - 1) * IGB_MAX_CHUNK_SIZE, approximately
4 MB, split into 4095 1 KB frames.  At 1 Gb/s line rate (~125 MB/s),
4 MB takes ~32 ms on the wire, plus per-frame preamble, SFD,
inter-frame gap and FCS overhead (~3%) and descriptor fetch/writeback
latency.  The 1 ms cap times out long before any valid transfer can
complete.

Wait up to ~200 ms (200 iterations * 1 ms) for descriptor writeback
before returning -ETIMEDOUT.  ~6x the line-rate floor leaves comfortable
headroom for host scheduling jitter while keeping the intentional
invalid-DMA tests (mix_and_match) so they recover quickly on real faults.

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Alex Williamson <alex.williamson@nvidia.com>
---
 .../testing/selftests/vfio/lib/drivers/igb/igb.c  | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
index a8f2072b6820..879c21780f5b 100644
--- a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
+++ b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
@@ -403,11 +403,22 @@ static int igb_memcpy_wait(struct vfio_pci_device *device)
 	prev_tail = (igb->rx_tail + RING_SIZE - 1) % RING_SIZE;
 	rx = &igb->rx_ring[prev_tail];
 
-	retries = 100;
+	/*
+	 * Real 82576 hardware processes the descriptor ring at line rate.
+	 * max_memcpy_size = (RING_SIZE - 1) * IGB_MAX_CHUNK_SIZE ~= 4 MB,
+	 * split into 4095 1 KB frames.  At 1 Gb/s (~125 MB/s) the worst
+	 * valid memcpy takes ~32 ms on the wire, plus per-frame preamble,
+	 * SFD, IFG and FCS overhead (~3%) and descriptor fetch/writeback
+	 * latency.  Wait up to ~200 ms before declaring the device hung;
+	 * ~6x the line-rate floor leaves comfortable headroom for host
+	 * scheduling jitter while keeping the intentional invalid-DMA
+	 * tests bounded.
+	 */
+	retries = 200;
 	while (retries-- > 0) {
 		if (rx->wb.status_error & 1)
 			break;
-		usleep(10);
+		usleep(1000);
 	}
 
 	igb_irq_clear(igb);
-- 
2.54.0.794.g4f17f83d09-goog

[PATCH v2 06/10] selftests/vfio: igb: Disable PCIe completion timeout retries

[Josh Hilke]

From: Alex Williamson <alex.williamson@nvidia.com>

The mix_and_match test intentionally submits TX descriptors with an
unmapped source IOVA so that the DMA read fails.  By default the 82576
re-sends the request after a PCIe completion timeout (datasheet section
8.6.1, GCR.Completion_Timeout_Resend, bit 16, initial value 1b).  On
real hardware this turns a single fault into a stream of retried reads,
keeping PCIe AER and IOMMU error handling busy and interfering with
reset recovery.

Clear GCR.Completion_Timeout_Resend during device initialization so a
failed read fails once and stays failed.

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Alex Williamson <alex.williamson@nvidia.com>
---
 tools/testing/selftests/vfio/lib/drivers/igb/igb.c   | 12 ++++++++++++
 .../selftests/vfio/lib/drivers/igb/registers.h       |  2 ++
 2 files changed, 14 insertions(+)

diff --git a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
index 879c21780f5b..89537f8345d1 100644
--- a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
+++ b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
@@ -209,6 +209,18 @@ static void igb_init(struct vfio_pci_device *device)
 		vfio_pci_config_writew(device, PCI_COMMAND, cmd_reg);
 	}
 
+	/*
+	 * Disable DMA re-send on PCIe completion timeout (82576 datasheet
+	 * section 8.6.1, GCR.Completion_Timeout_Resend, bit 16).  The
+	 * mix_and_match test intentionally submits descriptors targeting
+	 * unmapped IOVAs; with the default (set) value, the device keeps
+	 * retrying the failed read indefinitely, which keeps PCIe AER and
+	 * IOMMU error handling busy and interferes with reset recovery.
+	 */
+	ctrl = igb_read32(igb, IGB_GCR);
+	ctrl &= ~IGB_GCR_CMPL_TMOUT_RESEND;
+	igb_write32(igb, IGB_GCR, ctrl);
+
 	/* Configure PHY internal loopback for testing. */
 	if (igb_setup_loopback(igb))
 		return;
diff --git a/tools/testing/selftests/vfio/lib/drivers/igb/registers.h b/tools/testing/selftests/vfio/lib/drivers/igb/registers.h
index a133a208ed08..3f04bcaa44ba 100644
--- a/tools/testing/selftests/vfio/lib/drivers/igb/registers.h
+++ b/tools/testing/selftests/vfio/lib/drivers/igb/registers.h
@@ -73,6 +73,8 @@
 
 #define IGB_RAH0 0x05404 /* Receive Address High 0 */
 #define IGB_VMOLR0 0x05AD0 /* VM Offload Layout Register 0 */
+#define IGB_GCR 0x05B00 /* PCIe Control */
+#define IGB_GCR_CMPL_TMOUT_RESEND BIT(16) /* Re-send on completion timeout */
 
 #define IGB_VMOLR_LPE 0x00010000 /* Long Packet Enable */
 #define IGB_VMOLR_BAM 0x08000000 /* Broadcast Accept Mode */
-- 
2.54.0.794.g4f17f83d09-goog

[PATCH v2 07/10] selftests/vfio: Add vfio_pci_irq_reenable() helper

[Josh Hilke]

From: Alex Williamson <alex.williamson@nvidia.com>

Selftest drivers that recover from a fault by issuing VFIO_DEVICE_RESET
need to re-arm device interrupts afterwards.  VFIO_DEVICE_RESET tears
down the kernel-side IRQ trigger so a subsequent VFIO_DEVICE_SET_IRQS
is required, but the user-side eventfds (and any fd cached in a test
fixture) are still valid and must be preserved.

vfio_pci_irq_enable() refuses to be called for vectors that already
have an eventfd (VFIO_ASSERT_LT), and vfio_pci_irq_disable() closes
all eventfds before resetting the trigger, so neither is suitable.

Add vfio_pci_irq_reenable(device, index, vector, count) which asserts
that the requested range has existing eventfds and re-issues
VFIO_DEVICE_SET_IRQS using them.  Signature mirrors vfio_pci_irq_enable().

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Alex Williamson <alex.williamson@nvidia.com>
---
 .../lib/include/libvfio/vfio_pci_device.h     |  2 ++
 .../selftests/vfio/lib/vfio_pci_device.c      | 22 +++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h b/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
index 2858885a89bb..a362e2b2bfda 100644
--- a/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
+++ b/tools/testing/selftests/vfio/lib/include/libvfio/vfio_pci_device.h
@@ -68,6 +68,8 @@ void vfio_pci_config_access(struct vfio_pci_device *device, bool write,
 void vfio_pci_irq_enable(struct vfio_pci_device *device, u32 index,
 			 u32 vector, int count);
 void vfio_pci_irq_disable(struct vfio_pci_device *device, u32 index);
+void vfio_pci_irq_reenable(struct vfio_pci_device *device, u32 index,
+			   u32 vector, int count);
 void vfio_pci_irq_trigger(struct vfio_pci_device *device, u32 index, u32 vector);
 
 static inline void fcntl_set_nonblock(int fd)
diff --git a/tools/testing/selftests/vfio/lib/vfio_pci_device.c b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
index fc75e04ef010..7b8394d0ac50 100644
--- a/tools/testing/selftests/vfio/lib/vfio_pci_device.c
+++ b/tools/testing/selftests/vfio/lib/vfio_pci_device.c
@@ -106,6 +106,28 @@ void vfio_pci_irq_disable(struct vfio_pci_device *device, u32 index)
 	vfio_pci_irq_set(device, index, 0, 0, NULL);
 }
 
+/*
+ * Re-issue VFIO_DEVICE_SET_IRQS for an already-enabled vector range using
+ * the existing eventfds.  Intended for drivers that need to re-arm device
+ * interrupts after a VFIO_DEVICE_RESET, which tears down the kernel-side
+ * IRQ trigger but leaves user-side eventfds intact.  Recreating the
+ * eventfds would invalidate any test-fixture cache of the fd, so this
+ * helper deliberately preserves them.
+ */
+void vfio_pci_irq_reenable(struct vfio_pci_device *device, u32 index,
+			   u32 vector, int count)
+{
+	int i;
+
+	check_supported_irq_index(index);
+
+	for (i = vector; i < vector + count; i++)
+		VFIO_ASSERT_GE(device->msi_eventfds[i], 0,
+			       "vector %d eventfd not allocated\n", i);
+
+	vfio_pci_irq_set(device, index, vector, count, device->msi_eventfds + vector);
+}
+
 static void vfio_pci_irq_get(struct vfio_pci_device *device, u32 index,
 			     struct vfio_irq_info *irq_info)
 {
-- 
2.54.0.794.g4f17f83d09-goog

[PATCH v2 08/10] selftests/vfio: igb: Factor hardware programming into igb_hw_init()

[Josh Hilke]

From: Alex Williamson <alex.williamson@nvidia.com>

Split the device register programming out of igb_init() into a new
igb_hw_init() helper so that the same sequence can be re-run after a
VFIO_DEVICE_RESET to restore the registers that CTRL.RST clears.  No
functional change for the initial path.

igb_init() now performs the one-shot setup: region size assertion, BAR
mapping, CTRL.RST + IMC mask-all to put the device into a known state,
and vfio_pci_msix_enable() to set up the kernel-side IRQ trigger.
igb_hw_init() does the rest: ring pointer setup and IOVA calc,
CTRL_EXT, PCI bus master, GCR, PHY loopback, descriptor rings, RCTL,
TCTL, GPIE/EIAC/EIAM/EIMS/IVAR, and driver-state initialization.

vfio_pci_msix_enable() moves from after RCTL/TCTL to before all
device-side programming.  Its only side effects are the VFIO kernel
IRQ trigger setup and the PCI MSI-X capability bits in config space;
neither has any ordering dependency on the 82576 device register
writes performed in igb_hw_init().  Performing it once in igb_init()
keeps igb_hw_init() reusable from the reset recovery path (which uses
vfio_pci_irq_reenable() to re-arm the existing trigger).

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Alex Williamson <alex.williamson@nvidia.com>
---
 .../selftests/vfio/lib/drivers/igb/igb.c      | 44 +++++++++++++------
 1 file changed, 31 insertions(+), 13 deletions(-)

diff --git a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
index 89537f8345d1..1ac0a32523ea 100644
--- a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
+++ b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
@@ -175,7 +175,13 @@ static int igb_probe(struct vfio_pci_device *device)
 	return 0;
 }
 
-static void igb_init(struct vfio_pci_device *device)
+/*
+ * Program the device into a usable state.  Split out of igb_init() so it
+ * can be reused after a device reset to re-program the registers that
+ * CTRL.RST clears.  Expects bar0 to be mapped and MSI-X already enabled
+ * via VFIO.
+ */
+static void igb_hw_init(struct vfio_pci_device *device)
 {
 	struct igb *igb = to_igb_state(device);
 	u64 iova_tx, iova_rx;
@@ -183,19 +189,10 @@ static void igb_init(struct vfio_pci_device *device)
 	u16 cmd_reg;
 	int retries;
 
-	VFIO_ASSERT_GE(device->driver.region.size, sizeof(struct igb));
 
-	/* Set up rings and calculate IOVAs */
-	igb->bar0 = device->bars[0].vaddr;
 
 	iova_tx = to_iova(device, igb->tx_ring);
 	iova_rx = to_iova(device, igb->rx_ring);
-
-	/* Reset device and disable all interrupts */
-	igb_write32(igb, IGB_CTRL, igb_read32(igb, IGB_CTRL) | IGB_CTRL_RST);
-	usleep(20000);
-	igb_write32(igb, IGB_IMC, 0xFFFFFFFF);
-
 	/* Signal that the driver is loaded */
 	ctrl = igb_read32(igb, IGB_CTRL_EXT);
 	ctrl |= IGB_CTRL_EXT_DRV_LOAD;
@@ -280,9 +277,6 @@ static void igb_init(struct vfio_pci_device *device)
 	igb_write32(igb, IGB_RCTL, rctl);
 	igb_write32(igb, IGB_TCTL, IGB_TCTL_EN);
 
-	/* Enable MSI-X with 1 vector for the test */
-	vfio_pci_msix_enable(device, MSIX_VECTOR, 1);
-
 	/*
 	 * Program MSI-X interrupt routing per 82576 datasheet:
 	 *
@@ -322,7 +316,31 @@ static void igb_init(struct vfio_pci_device *device)
 	device->driver.msi = MSIX_VECTOR;
 }
 
+static void igb_init(struct vfio_pci_device *device)
+{
+	struct igb *igb = to_igb_state(device);
+
+	VFIO_ASSERT_GE(device->driver.region.size, sizeof(struct igb));
+
+	igb->bar0 = device->bars[0].vaddr;
+
+	/* Reset device and disable all interrupts. */
+	igb_write32(igb, IGB_CTRL, igb_read32(igb, IGB_CTRL) | IGB_CTRL_RST);
+	usleep(20000);
+	igb_write32(igb, IGB_IMC, 0xFFFFFFFF);
+
+	/*
+	 * Enable MSI-X via VFIO before device-side register programming.
+	 * vfio_pci_msix_enable() only touches the VFIO IRQ machinery and the
+	 * PCI MSI-X capability via config space; it has no ordering
+	 * dependency on the device-side writes performed by igb_hw_init().
+	 * Placing it here keeps igb_hw_init() reusable from the reset
+	 * recovery path (which calls vfio_pci_irq_reenable() instead).
+	 */
+	vfio_pci_msix_enable(device, MSIX_VECTOR, 1);
 
+	igb_hw_init(device);
+}
 static void igb_remove(struct vfio_pci_device *device)
 {
 	struct igb *igb = to_igb_state(device);
-- 
2.54.0.794.g4f17f83d09-goog

[PATCH v2 09/10] selftests/vfio: igb: Recover after DMA-read faults

[Josh Hilke]

From: Alex Williamson <alex.williamson@nvidia.com>

The mix_and_match test intentionally submits a TX descriptor with an
unmapped source IOVA so that the DMA read fails.  On real 82576
hardware the resulting fault leaves the descriptor engine unable to
service subsequent valid descriptors, so the next memcpy in the same
test iteration times out.

The 82576 datasheet (section 4.2.1.6.1) describes CTRL.RST as the
software mechanism to recover from a hung device.  Empirically
CTRL.RST alone is not sufficient in this state: the visible queue
registers are reinitialized, but the next valid memcpy still posts
descriptors without any TDH/TDT progress in the same process.  A
fresh device open after the failure works, which points to a reset
scope broader than CTRL.RST being required.  The 82576 advertises
PCIe FLR; VFIO_DEVICE_RESET drives FLR and supplies that scope while
preserving the selftest process and its DMA mappings.

Add igb_error_reset_and_reinit() implementing the recovery sequence:
issue VFIO_DEVICE_RESET, re-arm the kernel-side MSI-X trigger against
the still-valid eventfd via vfio_pci_irq_reenable() (this does not
touch the eventfd, which test fixtures may have cached), and
re-program the device via igb_hw_init().  FLR clears EICR and leaves
EIMS=0, so no explicit interrupt mask or cause writes are needed.
igb_hw_init() resets tx_tail/rx_tail to 0 and igb_memcpy_start() zeros
each descriptor before submission, so no ring memset is needed either.

Call this from igb_memcpy_wait() on completion timeout, preceded by a
10 ms delay so that PCIe/IOMMU/AER error handling triggered by the
just-observed DMA fault can release the device lock VFIO_DEVICE_RESET
contends for.  The delay is heuristic and tied to the fault path, so
it lives at the call site rather than inside the reset helper.  The
failed memcpy still returns -ETIMEDOUT; reset recovery only ensures
the next operation starts from a usable device state.

Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Alex Williamson <alex.williamson@nvidia.com>
---
 .../selftests/vfio/lib/drivers/igb/igb.c      | 40 +++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
index 1ac0a32523ea..5568b463c2f2 100644
--- a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
+++ b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
@@ -423,6 +423,28 @@ static void igb_memcpy_start(struct vfio_pci_device *device, iova_t src,
 	igb_write32(igb, IGB_TDT0, igb->tx_tail);
 }
 
+/*
+ * Reset the device via VFIO_DEVICE_RESET (PCIe FLR on the 82576) and
+ * re-program it.  VFIO_DEVICE_RESET tears down the kernel-side MSI-X
+ * trigger but leaves user-side eventfds intact, so re-arm the trigger
+ * via vfio_pci_irq_reenable() before reprogramming so any caller-cached
+ * eventfd remains valid.
+ *
+ * FLR clears device-side state to power-on reset values (datasheet
+ * 4.2.1.5.1: a PF FLR is "equivalent to a D0->D3->D0 transition"), so
+ * EIMS and EICR come back as 0 from their register-defined initial
+ * values, and igb_hw_init() resets tx_tail/rx_tail to 0.  The next
+ * igb_memcpy_start() will memset each descriptor it touches before
+ * submission, so no explicit IMC/EICR writes or ring memsets are
+ * needed here.
+ */
+static void igb_error_reset_and_reinit(struct vfio_pci_device *device)
+{
+	vfio_pci_device_reset(device);
+	vfio_pci_irq_reenable(device, VFIO_PCI_MSIX_IRQ_INDEX, MSIX_VECTOR, 1);
+	igb_hw_init(device);
+}
+
 static int igb_memcpy_wait(struct vfio_pci_device *device)
 {
 	struct igb *igb = to_igb_state(device);
@@ -458,6 +480,24 @@ static int igb_memcpy_wait(struct vfio_pci_device *device)
 	if (rx->wb.status_error & 1)
 		return 0;
 
+	/*
+	 * The descriptor never completed.  On real 82576 hardware this
+	 * typically follows a DMA-read fault from one of the intentional
+	 * unmapped-IOVA tests; the fault leaves the descriptor engine
+	 * unable to service subsequent valid descriptors.  CTRL.RST alone
+	 * reinitializes the queue registers but leaves the engine wedged
+	 * for the current process, so a broader VFIO_DEVICE_RESET (FLR)
+	 * is required.
+	 *
+	 * Delay before requesting reset so PCIe/IOMMU/AER error handling
+	 * triggered by the just-observed DMA fault can release the device
+	 * lock VFIO_DEVICE_RESET contends for.  The 10 ms value is
+	 * heuristic.  The current memcpy still fails with -ETIMEDOUT;
+	 * recovery only ensures the next memcpy starts from a usable state.
+	 */
+	usleep(10000);
+	igb_error_reset_and_reinit(device);
+
 	return -ETIMEDOUT;
 }
 
-- 
2.54.0.794.g4f17f83d09-goog

[PATCH v2 10/10] selftests/vfio: igb: Use offical IGB headers in selftest driver

[Josh Hilke]

Include the offical Intel network driver header files in
tools/testing/selftests/vfio/lib/drivers/igb/igb.c and refactor the
driver to directly utilize standard E1000_ prefixed registers and macros
found from those headers.

Delete tools/testing/selftests/vfio/lib/drivers/igb/registers.h as it
only contains duplicate code found in the offical IGB header files.

Assisted-by: Gemini:gemini-3.1-pro-preview
Suggested-by: David Matlack <dmatlack@google.com>
Signed-off-by: Josh Hilke <jrhilke@google.com>

# Conflicts:
#	tools/testing/selftests/vfio/lib/drivers/igb/igb.c
---
 .../selftests/vfio/lib/drivers/igb/igb.c      | 159 +++++++++---------
 .../vfio/lib/drivers/igb/registers.h          | 127 --------------
 tools/testing/selftests/vfio/lib/libvfio.mk   |   1 +
 3 files changed, 83 insertions(+), 204 deletions(-)
 delete mode 100644 tools/testing/selftests/vfio/lib/drivers/igb/registers.h

diff --git a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
index 5568b463c2f2..74b423954554 100644
--- a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
+++ b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
@@ -5,14 +5,19 @@
 #include <linux/io.h>
 #include <linux/pci_regs.h>
 #include <linux/pci_ids.h>
+#include <linux/mii.h>
 #include <libvfio/vfio_pci_device.h>
 
-#include "registers.h"
+#include "e1000_regs.h"
+#include "e1000_defines.h"
+#include "e1000_82575.h"
 
 #define PCI_DEVICE_ID_INTEL_82576 0x10C9
 #define IGB_MAX_CHUNK_SIZE 1024
 #define MSIX_VECTOR 0
 #define RING_SIZE 4096 /* Number of descriptors in ring */
+#define IGB_ETH_OVERHEAD_SZ 18
+#define IGB_JUMBO_FRAME_SIZE (IGB_MAX_CHUNK_SIZE + IGB_ETH_OVERHEAD_SZ)
 
 struct igb_tx_desc {
 	union {
@@ -76,23 +81,23 @@ static int igb_write_phy(struct igb *igb, u32 offset, u16 data)
 	int i;
 
 	mdic = (((u32)data) |
-		(offset << IGB_MDIC_REG_SHIFT) |
-		(1 << IGB_MDIC_PHY_SHIFT) |
-		IGB_MDIC_OP_WRITE);
+		(offset << E1000_MDIC_REG_SHIFT) |
+		(1 << E1000_MDIC_PHY_SHIFT) |
+		E1000_MDIC_OP_WRITE);
 
-	igb_write32(igb, IGB_MDIC, mdic);
+	igb_write32(igb, E1000_MDIC, mdic);
 
 	for (i = 0; i < 1000; i++) {
 		usleep(50);
-		mdic = igb_read32(igb, IGB_MDIC);
-		if (mdic & IGB_MDIC_READY)
+		mdic = igb_read32(igb, E1000_MDIC);
+		if (mdic & E1000_MDIC_READY)
 			break;
 	}
 
-	if (!(mdic & IGB_MDIC_READY))
+	if (!(mdic & E1000_MDIC_READY))
 		return -1;
 
-	if (mdic & IGB_MDIC_ERROR)
+	if (mdic & E1000_MDIC_ERROR)
 		return -1;
 
 	return 0;
@@ -125,14 +130,14 @@ static int igb_setup_loopback(struct igb *igb)
 	 * below clears the autoneg-enable bit, so this is effectively a
 	 * no-op there.
 	 */
-	(void)igb_write_phy(igb, IGB_PHY_CTRL_REG_OFFSET,
-			    IGB_PHY_CTRL_AN_ENABLE | IGB_PHY_CTRL_AN_RESTART);
+	(void)igb_write_phy(igb, MII_BMCR,
+			    BMCR_ANENABLE | BMCR_ANRESTART);
 
 	/* PHY control: loopback + 1Gb/s full duplex, autoneg disabled. */
-	ret = igb_write_phy(igb, IGB_PHY_CTRL_REG_OFFSET,
-			    IGB_PHY_CTRL_LOOPBACK |
-			    IGB_PHY_CTRL_SPEED_1000 |
-			    IGB_PHY_CTRL_FULL_DUPLEX);
+	ret = igb_write_phy(igb, MII_BMCR,
+			    BMCR_LOOPBACK |
+			    BMCR_SPEED1000 |
+			    BMCR_FULLDPLX);
 	if (ret)
 		return ret;
 
@@ -148,14 +153,14 @@ static int igb_setup_loopback(struct igb *igb)
 	 * the link state the descriptor engine does not run, since the chip
 	 * normally waits for a real negotiated link.
 	 */
-	ctrl = igb_read32(igb, IGB_CTRL);
-	ctrl &= ~IGB_CTRL_SPD_SEL;
-	ctrl |= IGB_CTRL_FRCSPD |
-		IGB_CTRL_FRCDPX |
-		IGB_CTRL_SPD_1000 |
-		IGB_CTRL_FD |
-		IGB_CTRL_SLU;
-	igb_write32(igb, IGB_CTRL, ctrl);
+	ctrl = igb_read32(igb, E1000_CTRL);
+	ctrl &= ~E1000_CTRL_SPD_SEL;
+	ctrl |= E1000_CTRL_FRCSPD |
+		E1000_CTRL_FRCDPX |
+		E1000_CTRL_SPD_1000 |
+		E1000_CTRL_FD |
+		E1000_CTRL_SLU;
+	igb_write32(igb, E1000_CTRL, ctrl);
 
 	/*
 	 * Settling delay matching the kernel ethtool selftest's msleep(500)
@@ -189,15 +194,14 @@ static void igb_hw_init(struct vfio_pci_device *device)
 	u16 cmd_reg;
 	int retries;
 
-
-
 	iova_tx = to_iova(device, igb->tx_ring);
 	iova_rx = to_iova(device, igb->rx_ring);
+
 	/* Signal that the driver is loaded */
-	ctrl = igb_read32(igb, IGB_CTRL_EXT);
-	ctrl |= IGB_CTRL_EXT_DRV_LOAD;
-	ctrl &= ~IGB_CTRL_EXT_LINK_MODE_MASK;
-	igb_write32(igb, IGB_CTRL_EXT, ctrl);
+	ctrl = igb_read32(igb, E1000_CTRL_EXT);
+	ctrl |= E1000_CTRL_EXT_DRV_LOAD;
+	ctrl &= ~E1000_CTRL_EXT_LINK_MODE_MASK;
+	igb_write32(igb, E1000_CTRL_EXT, ctrl);
 
 	/* Enable PCI Bus Master. */
 	cmd_reg = vfio_pci_config_readw(device, PCI_COMMAND);
@@ -214,27 +218,27 @@ static void igb_hw_init(struct vfio_pci_device *device)
 	 * retrying the failed read indefinitely, which keeps PCIe AER and
 	 * IOMMU error handling busy and interferes with reset recovery.
 	 */
-	ctrl = igb_read32(igb, IGB_GCR);
-	ctrl &= ~IGB_GCR_CMPL_TMOUT_RESEND;
-	igb_write32(igb, IGB_GCR, ctrl);
+	ctrl = igb_read32(igb, E1000_GCR);
+	ctrl &= ~E1000_GCR_CMPL_TMOUT_RESEND;
+	igb_write32(igb, E1000_GCR, ctrl);
 
 	/* Configure PHY internal loopback for testing. */
 	if (igb_setup_loopback(igb))
 		return;
 
 	/* Configure TX and RX descriptor rings */
-	igb_write32(igb, IGB_TDBAL0, (u32)iova_tx);
-	igb_write32(igb, IGB_TDBAH0, (u32)(iova_tx >> 32));
-	igb_write32(igb, IGB_TDLEN0, RING_SIZE * sizeof(struct igb_tx_desc));
-	igb_write32(igb, IGB_TDH0, 0);
-	igb_write32(igb, IGB_TDT0, 0);
-	igb_write32(igb, IGB_TXDCTL0, IGB_TXDCTL0_Q_EN);
-
-	igb_write32(igb, IGB_RDBAL0, (u32)iova_rx);
-	igb_write32(igb, IGB_RDBAH0, (u32)(iova_rx >> 32));
-	igb_write32(igb, IGB_RDLEN0, RING_SIZE * sizeof(struct igb_rx_desc));
-	igb_write32(igb, IGB_RDH0, 0);
-	igb_write32(igb, IGB_RDT0, 0);
+	igb_write32(igb, E1000_TDBAL(0), (u32)iova_tx);
+	igb_write32(igb, E1000_TDBAH(0), (u32)(iova_tx >> 32));
+	igb_write32(igb, E1000_TDLEN(0), RING_SIZE * sizeof(struct igb_tx_desc));
+	igb_write32(igb, E1000_TDH(0), 0);
+	igb_write32(igb, E1000_TDT(0), 0);
+	igb_write32(igb, E1000_TXDCTL(0), E1000_TXDCTL_QUEUE_ENABLE);
+
+	igb_write32(igb, E1000_RDBAL(0), (u32)iova_rx);
+	igb_write32(igb, E1000_RDBAH(0), (u32)(iova_rx >> 32));
+	igb_write32(igb, E1000_RDLEN(0), RING_SIZE * sizeof(struct igb_rx_desc));
+	igb_write32(igb, E1000_RDH(0), 0);
+	igb_write32(igb, E1000_RDT(0), 0);
 
 	/*
 	 * Select the advanced one-buffer descriptor format.  Per 82576
@@ -244,15 +248,15 @@ static void igb_hw_init(struct vfio_pci_device *device)
 	 * writeback layout (section 7.1.5.2), so polling rx.wb.status_error
 	 * requires this format.  Section 8.10.2 specifies DESCTYPE[27:25].
 	 */
-	igb_write32(igb, IGB_SRRCTL0, IGB_SRRCTL_DESCTYPE_ADV_ONEBUF);
+	igb_write32(igb, E1000_SRRCTL(0), E1000_SRRCTL_DESCTYPE_ADV_ONEBUF);
 
-	igb_write32(igb, IGB_RXDCTL0, IGB_RXDCTL0_Q_EN);
+	igb_write32(igb, E1000_RXDCTL(0), E1000_RXDCTL_QUEUE_ENABLE);
 
 	/* Wait for TX and RX queues to be enabled */
 	retries = 2000;
 	while (retries-- > 0) {
-		if ((igb_read32(igb, IGB_TXDCTL0) & IGB_TXDCTL0_Q_EN) &&
-		    (igb_read32(igb, IGB_RXDCTL0) & IGB_RXDCTL0_Q_EN))
+		if ((igb_read32(igb, E1000_TXDCTL(0)) & E1000_TXDCTL_QUEUE_ENABLE) &&
+		    (igb_read32(igb, E1000_RXDCTL(0)) & E1000_RXDCTL_QUEUE_ENABLE))
 			break;
 		usleep(10);
 	}
@@ -270,13 +274,12 @@ static void igb_hw_init(struct vfio_pci_device *device)
 	 * selftest work on both real hardware and QEMU without conditional
 	 * code paths.
 	 */
-	rctl = IGB_RCTL_EN |       /* Receiver Enable */
-	       IGB_RCTL_UPE |      /* Unicast Promiscuous (for dummy MAC) */
-	       IGB_RCTL_LBM_MAC |  /* MAC Loopback - for QEMU emulation only */
-	       IGB_RCTL_SECRC;     /* Strip CRC (needed for memcmp) */
-	igb_write32(igb, IGB_RCTL, rctl);
-	igb_write32(igb, IGB_TCTL, IGB_TCTL_EN);
-
+	rctl = E1000_RCTL_EN |       /* Receiver Enable */
+	       E1000_RCTL_UPE |      /* Unicast Promiscuous (for dummy MAC) */
+	       E1000_RCTL_LBM_MAC |  /* MAC Loopback - for QEMU emulation only */
+	       E1000_RCTL_SECRC;     /* Strip CRC (needed for memcmp) */
+	igb_write32(igb, E1000_RCTL, rctl);
+	igb_write32(igb, E1000_TCTL, E1000_TCTL_EN);
 	/*
 	 * Program MSI-X interrupt routing per 82576 datasheet:
 	 *
@@ -297,15 +300,15 @@ static void igb_hw_init(struct vfio_pci_device *device)
 	 * IVAR (section 7.3.1.2, register definition in 8.8.13): map RX
 	 * cause 0 to MSI-X vector 0 and mark the entry valid.
 	 */
-	igb_write32(igb, IGB_GPIE, IGB_GPIE_MULTIPLE_MSIX | IGB_GPIE_EIAME);
-	igb_write32(igb, IGB_EIAC, IGB_EICR_VEC0);
-	igb_write32(igb, IGB_EIAM, IGB_EICR_VEC0);
+	igb_write32(igb, E1000_GPIE, E1000_GPIE_MSIX_MODE | E1000_GPIE_EIAME);
+	igb_write32(igb, E1000_EIAC, E1000_EICR_RX_QUEUE0);
+	igb_write32(igb, E1000_EIAM, E1000_EICR_RX_QUEUE0);
 
 	/* Enable interrupts on vector 0 */
-	igb_write32(igb, IGB_EIMS, IGB_EICR_VEC0);
+	igb_write32(igb, E1000_EIMS, E1000_EICR_RX_QUEUE0);
 
 	/* Map vector 0 to interrupt cause 0 and mark it valid */
-	igb_write32(igb, IGB_IVAR0, IGB_IVAR_VALID);
+	igb_write32(igb, E1000_IVAR0, E1000_IVAR_VALID);
 
 	/* Initialize driver state and capability limits */
 	igb->tx_tail = 0;
@@ -316,6 +319,7 @@ static void igb_hw_init(struct vfio_pci_device *device)
 	device->driver.msi = MSIX_VECTOR;
 }
 
+
 static void igb_init(struct vfio_pci_device *device)
 {
 	struct igb *igb = to_igb_state(device);
@@ -325,9 +329,9 @@ static void igb_init(struct vfio_pci_device *device)
 	igb->bar0 = device->bars[0].vaddr;
 
 	/* Reset device and disable all interrupts. */
-	igb_write32(igb, IGB_CTRL, igb_read32(igb, IGB_CTRL) | IGB_CTRL_RST);
+	igb_write32(igb, E1000_CTRL, igb_read32(igb, E1000_CTRL) | E1000_CTRL_RST);
 	usleep(20000);
-	igb_write32(igb, IGB_IMC, 0xFFFFFFFF);
+	igb_write32(igb, E1000_IMC, 0xFFFFFFFF);
 
 	/*
 	 * Enable MSI-X via VFIO before device-side register programming.
@@ -341,24 +345,26 @@ static void igb_init(struct vfio_pci_device *device)
 
 	igb_hw_init(device);
 }
+
+
 static void igb_remove(struct vfio_pci_device *device)
 {
 	struct igb *igb = to_igb_state(device);
 
 	vfio_pci_msix_disable(device);
-	igb_write32(igb, IGB_RCTL, 0);
-	igb_write32(igb, IGB_TCTL, 0);
-	igb_write32(igb, IGB_CTRL, igb_read32(igb, IGB_CTRL) | IGB_CTRL_RST);
+	igb_write32(igb, E1000_RCTL, 0);
+	igb_write32(igb, E1000_TCTL, 0);
+	igb_write32(igb, E1000_CTRL, igb_read32(igb, E1000_CTRL) | E1000_CTRL_RST);
 }
 
 static void igb_irq_disable(struct igb *igb)
 {
-	igb_write32(igb, IGB_EIMC, IGB_EICR_VEC0);
+	igb_write32(igb, E1000_EIMC, E1000_EICR_RX_QUEUE0);
 }
 
 static void igb_irq_enable(struct igb *igb)
 {
-	igb_write32(igb, IGB_EIMS, IGB_EICR_VEC0);
+	igb_write32(igb, E1000_EIMS, E1000_EICR_RX_QUEUE0);
 }
 
 static void igb_irq_clear(struct igb *igb)
@@ -370,7 +376,7 @@ static void igb_irq_clear(struct igb *igb)
 	 * the read-to-clear path in 7.3.4.3.  Bits not in EIAC are still
 	 * cleared by writing 1.
 	 */
-	igb_write32(igb, IGB_EICR, 0xFFFFFFFF);
+	igb_write32(igb, E1000_EICR, 0xFFFFFFFF);
 }
 
 static void igb_memcpy_start(struct vfio_pci_device *device, iova_t src,
@@ -404,23 +410,23 @@ static void igb_memcpy_start(struct vfio_pci_device *device, iova_t src,
 		 * in bits 15:0 of cmd_type_len.
 		 */
 		tx->read.cmd_type_len = (uint32_t)size |
-			IGB_ADVTXD_DTYP_DATA |
-			IGB_ADVTXD_DCMD_DEXT |
-			IGB_ADVTXD_DCMD_IFCS |
-			IGB_ADVTXD_DCMD_EOP;
+			E1000_ADVTXD_DTYP_DATA |
+			E1000_ADVTXD_DCMD_DEXT |
+			E1000_ADVTXD_DCMD_IFCS |
+			E1000_ADVTXD_DCMD_EOP;
 		/*
 		 * PAYLEN (section 7.2.2.3.11) is the total payload size
 		 * in olinfo_status[31:14].
 		 */
 		tx->read.olinfo_status =
-			(uint32_t)size << IGB_ADVTXD_PAYLEN_SHIFT;
+			(uint32_t)size << E1000_ADVTXD_PAYLEN_SHIFT;
 
 		igb->tx_tail = (igb->tx_tail + 1) % RING_SIZE;
 		igb->rx_tail = (igb->rx_tail + 1) % RING_SIZE;
 	}
 
-	igb_write32(igb, IGB_RDT0, igb->rx_tail);
-	igb_write32(igb, IGB_TDT0, igb->tx_tail);
+	igb_write32(igb, E1000_RDT(0), igb->rx_tail);
+	igb_write32(igb, E1000_TDT(0), igb->tx_tail);
 }
 
 /*
@@ -474,7 +480,6 @@ static int igb_memcpy_wait(struct vfio_pci_device *device)
 	}
 
 	igb_irq_clear(igb);
-
 	igb_irq_enable(igb);
 
 	if (rx->wb.status_error & 1)
@@ -505,7 +510,7 @@ static void igb_send_msi(struct vfio_pci_device *device)
 {
 	struct igb *igb = to_igb_state(device);
 
-	igb_write32(igb, IGB_EICS, 1);
+	igb_write32(igb, E1000_EICS, 1);
 }
 
 const struct vfio_pci_driver_ops igb_ops = {
diff --git a/tools/testing/selftests/vfio/lib/drivers/igb/registers.h b/tools/testing/selftests/vfio/lib/drivers/igb/registers.h
deleted file mode 100644
index 3f04bcaa44ba..000000000000
--- a/tools/testing/selftests/vfio/lib/drivers/igb/registers.h
+++ /dev/null
@@ -1,127 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-
-#ifndef _IGB_REGISTERS_H_
-#define _IGB_REGISTERS_H_
-
-#include <linux/bits.h>
-
-/* Register Offsets (Intel 82576EB Datasheet) */
-#define IGB_CTRL 0x00000 /* Device Control */
-#define IGB_STATUS 0x00008 /* Device Status */
-#define IGB_CTRL_EXT 0x00018 /* Extended Device Control */
-#define IGB_MDIC 0x00020 /* MDI Control */
-#define IGB_RCTL 0x00100 /* Receive Control */
-#define IGB_TCTL 0x00400 /* Transmit Control */
-#define IGB_SCTL 0x00420 /* SerDes Control */
-
-/* Interrupt Registers */
-#define IGB_IMC 0x0150C /* Interrupt Mask Clear */
-#define IGB_IVAR0 0x01700 /* Interrupt Vector Allocation Register 0 */
-
-/* Rx Ring 0 Registers */
-#define IGB_RDBAL0 0x0C000 /* Rx Desc Base Address Low */
-#define IGB_RDBAH0 0x0C004 /* Rx Desc Base Address High */
-#define IGB_RDLEN0 0x0C008 /* Rx Desc Length */
-#define IGB_SRRCTL0 0x0C00C /* Split and Replication Receive Control Q0 */
-#define IGB_RDH0 0x0C010 /* Rx Desc Head */
-#define IGB_RDT0 0x0C018 /* Rx Desc Tail */
-#define IGB_RXDCTL0 0x0C028 /* Rx Desc Control */
-
-/* SRRCTL fields per 82576 datasheet section 8.10.2 */
-#define IGB_SRRCTL_DESCTYPE_ADV_ONEBUF (1u << 25) /* 001b: advanced one-buffer */
-
-/* Tx Ring 0 Registers */
-#define IGB_TDBAL0 0x0E000 /* Tx Desc Base Address Low */
-#define IGB_TDBAH0 0x0E004 /* Tx Desc Base Address High */
-#define IGB_TDLEN0 0x0E008 /* Tx Desc Length */
-#define IGB_TDH0 0x0E010 /* Tx Desc Head */
-#define IGB_TDT0 0x0E018 /* Tx Desc Tail */
-#define IGB_TXDCTL0 0x0E028 /* Tx Desc Control */
-
-/* Control Bit Definitions */
-/* CTRL */
-#define IGB_CTRL_FD		(1 << 0)  /* Full Duplex */
-#define IGB_CTRL_SLU		(1 << 6)  /* Set Link Up */
-#define IGB_CTRL_SPD_SEL	(3 << 8) /* Speed Select Mask */
-#define IGB_CTRL_SPD_1000	(2 << 8) /* Force 1000 Mb/s */
-#define IGB_CTRL_FRCSPD		(1 << 11) /* Force Speed */
-#define IGB_CTRL_FRCDPX		(1 << 12) /* Force Duplex */
-#define IGB_CTRL_RST		(1 << 26) /* Device Reset */
-#define IGB_CTRL_EXT_LINK_MODE_MASK (3 << 22)
-
-/* CTRL_EXT */
-#define IGB_CTRL_EXT_DRV_LOAD (1 << 28) /* Driver Loaded */
-
-/* RCTL */
-#define IGB_RCTL_EN (1 << 1) /* Receiver Enable */
-#define IGB_RCTL_UPE (1 << 3) /* Unicast Promiscuous Enabled */
-#define IGB_RCTL_LPE (1 << 5) /* Long Packet Reception Enable */
-#define IGB_RCTL_LBM_MAC (1 << 6) /* Loopback Mode - MAC (set as QEMU-only accommodation) */
-#define IGB_RCTL_SECRC (1 << 26) /* Strip Ethernet CRC */
-
-/* TCTL */
-#define IGB_TCTL_EN (1 << 1) /* Transmit Enable */
-
-/* SCTL */
-#define IGB_SCTL_DISABLE_SERDES_LOOPBACK (1 << 6)
-
-/* MDIC */
-#define IGB_MDIC_OP_WRITE (1 << 26)
-#define IGB_MDIC_OP_READ  (2 << 26)
-
-#define IGB_EICR 0x01580 /* Extended Interrupt Cause Read */
-
-#define IGB_RAH0 0x05404 /* Receive Address High 0 */
-#define IGB_VMOLR0 0x05AD0 /* VM Offload Layout Register 0 */
-#define IGB_GCR 0x05B00 /* PCIe Control */
-#define IGB_GCR_CMPL_TMOUT_RESEND BIT(16) /* Re-send on completion timeout */
-
-#define IGB_VMOLR_LPE 0x00010000 /* Long Packet Enable */
-#define IGB_VMOLR_BAM 0x08000000 /* Broadcast Accept Mode */
-#define IGB_RAH_POOL_1 0x00040000 /* Pool 1 assignment */
-
-#define IGB_EICS 0x01520 /* Extended Interrupt Cause Set */
-#define IGB_EIMS 0x01524 /* Extended Interrupt Mask Set */
-#define IGB_EIMC 0x01528 /* Extended Interrupt Mask Clear */
-#define IGB_EIAC 0x0152C /* Extended Interrupt Auto Clear */
-#define IGB_EIAM 0x01530 /* Extended Interrupt Auto Mask Enable */
-#define IGB_EICR_VEC0 BIT(0) /* MSI-X cause/vector 0 */
-#define IGB_CTRL_GIO_MASTER_DISABLE (1 << 2) /* GIO Master Disable */
-#define IGB_STATUS_GIO_MASTER_ENABLE (1 << 19) /* GIO Master Enable */
-#define IGB_GPIE 0x01514 /* General Purpose Interrupt Enable */
-/* GPIE fields per 82576 datasheet section 7.3.2.11, Table 7-47 */
-#define IGB_GPIE_MULTIPLE_MSIX BIT(4)  /* Multi-vector MSI-X mode */
-#define IGB_GPIE_EIAME         BIT(30) /* Apply EIAM on MSI-X assertion */
-#define IGB_TXDCTL0_Q_EN (1 << 25) /* Transmit Queue Enable */
-#define IGB_RXDCTL0_Q_EN (1 << 25) /* Receive Queue Enable */
-#define IGB_MRQC 0x05818 /* Multiple Receive Queues Command */
-
-#define IGB_MDIC_PHY_SHIFT 21 /* PHY Address Shift */
-#define IGB_MDIC_REG_SHIFT 16 /* Register Address Shift */
-#define IGB_MDIC_READY (1 << 28) /* MDI Data Ready */
-#define IGB_MDIC_ERROR (1 << 29) /* MDI Error */
-
-/* PHY register 0 (Control), per 82576 datasheet section 3.5.6.3.1 */
-#define IGB_PHY_CTRL_REG_OFFSET		0
-#define IGB_PHY_CTRL_AN_RESTART		0x0200 /* bit 9 */
-#define IGB_PHY_CTRL_AN_ENABLE		0x1000 /* bit 12 */
-#define IGB_PHY_CTRL_SPEED_1000		0x0040 /* bit 6 set, bit 13 clear */
-#define IGB_PHY_CTRL_FULL_DUPLEX	0x0100 /* bit 8 */
-#define IGB_PHY_CTRL_LOOPBACK		0x4000 /* bit 14 */
-
-#define IGB_IVAR_VALID 0x80 /* Valid bit for IVAR register */
-
-/*
- * Advanced TX Data Descriptor fields per 82576 datasheet section 7.2.2.3.
- * The cmd_type_len word holds: DTALEN[15:0], MAC[19:18], DTYP[23:20],
- * DCMD[31:24].  The olinfo_status word holds: STA[3:0], IDX[6:4],
- * POPTS[13:8], PAYLEN[31:14].
- */
-#define IGB_ADVTXD_DTYP_DATA	(0x3u << 20) /* DTYP=0011b: advanced data */
-#define IGB_ADVTXD_DCMD_EOP	(1u << 24)   /* DCMD bit 0: End of Packet */
-#define IGB_ADVTXD_DCMD_IFCS	(1u << 25)   /* DCMD bit 1: Insert FCS */
-#define IGB_ADVTXD_DCMD_RS	(1u << 27)   /* DCMD bit 3: Report Status */
-#define IGB_ADVTXD_DCMD_DEXT	(1u << 29)   /* DCMD bit 5: 1b for advanced */
-#define IGB_ADVTXD_PAYLEN_SHIFT	14           /* PAYLEN bit position */
-
-#endif /* _IGB_REGISTERS_H_ */
diff --git a/tools/testing/selftests/vfio/lib/libvfio.mk b/tools/testing/selftests/vfio/lib/libvfio.mk
index 1f13cca04348..0075749e2f12 100644
--- a/tools/testing/selftests/vfio/lib/libvfio.mk
+++ b/tools/testing/selftests/vfio/lib/libvfio.mk
@@ -23,6 +23,7 @@ LIBVFIO_O_DIRS := $(shell dirname $(LIBVFIO_O) | uniq)
 $(shell mkdir -p $(LIBVFIO_O_DIRS))
 
 CFLAGS += -I$(LIBVFIO_SRCDIR)/include
+CFLAGS += -I$(top_srcdir)/drivers/net/ethernet/intel/igb
 
 $(LIBVFIO_O): $(LIBVFIO_OUTPUT)/%.o : $(LIBVFIO_SRCDIR)/%.c
 	$(CC) $(CFLAGS) $(CPPFLAGS) $(TARGET_ARCH) -c $< -o $@
-- 
2.54.0.794.g4f17f83d09-goog

Re: [PATCH v2 10/10] selftests/vfio: igb: Use offical IGB headers in selftest driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Low] Unused macros IGB_ETH_OVERHEAD_SZ and IGB_JUMBO_FRAME_SIZE were introduced.
- [Low] Including `e1000_82575.h` without forward declaring `struct e1000_hw` may cause compiler warnings.
- [Low] The patch uses E1000_EICR_RX_QUEUE0 (a legacy Rx cause mask) to represent an MSI-X vector bit in interrupt mask/clear registers.
--

commit bff1218f00385f0c7ad23675f49b73f63118bf25
Author: Josh Hilke <jrhilke@google.com>

selftests/vfio: igb: Use offical IGB headers in selftest driver

This commit includes the official Intel network driver header files in
the igb selftest and refactors the driver to directly utilize standard
E1000_ prefixed registers and macros. It also removes the duplicated
registers.h file.

> diff --git a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
> index 5568b463c2f2b..74b4239545541 100644
> --- a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
> +++ b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
> @@ -5,14 +5,19 @@
>  #include <linux/io.h>
>  #include <linux/pci_regs.h>
>  #include <linux/pci_ids.h>
> +#include <linux/mii.h>
>  #include <libvfio/vfio_pci_device.h>
>  
> -#include "registers.h"
> +#include "e1000_regs.h"
> +#include "e1000_defines.h"
> +#include "e1000_82575.h"

[Severity: Low]
Since e1000_82575.h contains function prototypes that reference struct e1000_hw,
which is not declared here, will this cause compiler warnings (like "struct
e1000_hw declared inside parameter list")? If the selftests are built with
-Werror, could this result in a build failure?

>  
>  #define PCI_DEVICE_ID_INTEL_82576 0x10C9
>  #define IGB_MAX_CHUNK_SIZE 1024
>  #define MSIX_VECTOR 0
>  #define RING_SIZE 4096 /* Number of descriptors in ring */
> +#define IGB_ETH_OVERHEAD_SZ 18
> +#define IGB_JUMBO_FRAME_SIZE (IGB_MAX_CHUNK_SIZE + IGB_ETH_OVERHEAD_SZ)

[Severity: Low]
Are these new macros used anywhere in the code? It looks like they might be dead
code since they aren't referenced.

[ ... ]

> @@ -297,15 +300,15 @@ static void igb_hw_init(struct vfio_pci_device *device)
>  	 * IVAR (section 7.3.1.2, register definition in 8.8.13): map RX
>  	 * cause 0 to MSI-X vector 0 and mark the entry valid.
>  	 */
> -	igb_write32(igb, IGB_GPIE, IGB_GPIE_MULTIPLE_MSIX | IGB_GPIE_EIAME);
> -	igb_write32(igb, IGB_EIAC, IGB_EICR_VEC0);
> -	igb_write32(igb, IGB_EIAM, IGB_EICR_VEC0);
> +	igb_write32(igb, E1000_GPIE, E1000_GPIE_MSIX_MODE | E1000_GPIE_EIAME);
> +	igb_write32(igb, E1000_EIAC, E1000_EICR_RX_QUEUE0);
> +	igb_write32(igb, E1000_EIAM, E1000_EICR_RX_QUEUE0);
>  
>  	/* Enable interrupts on vector 0 */
> -	igb_write32(igb, IGB_EIMS, IGB_EICR_VEC0);
> +	igb_write32(igb, E1000_EIMS, E1000_EICR_RX_QUEUE0);

[Severity: Low]
Is it semantically correct to use E1000_EICR_RX_QUEUE0 here?

In MSI-X mode, the bits in EIMS, EIMC, EIAC, and EIAM correspond directly to
MSI-X vectors (where vector 0 is bit 0), rather than legacy interrupt causes.
While E1000_EICR_RX_QUEUE0 equals BIT(0) and works mathematically, using a
legacy cause mask to represent an MSI-X vector bit seems confusing.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260526235417.2058313-1-jrhilke@google.com?part=10