On Thu, 30 Jul 2026 23:31:29 +0000
Josh Hilke <[email protected]> wrote:
> +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);
> +
> + igb_reset(igb);
> +
> + /* Signal that the driver is loaded */
> + 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);
> + if ((cmd_reg & (PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY)) !=
> + (PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY)) {
> + cmd_reg |= (PCI_COMMAND_MASTER | PCI_COMMAND_MEMORY);
> + vfio_pci_config_writew(device, PCI_COMMAND, cmd_reg);
> + }
> +
> + /* Configure PHY internal loopback for testing. */
> + igb_setup_loopback(igb);
> +
> + /*
> + * 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, E1000_GCR);
> + ctrl &= ~E1000_GCR_CMPL_TMOUT_RESEND;
> + igb_write32(igb, E1000_GCR, ctrl);
> +
> + /* Configure TX and RX descriptor rings */
> + 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
> + * 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].
> + *
> + * The direct write also zeroes SRRCTL.BSIZEPACKET, which is
> + * intentional: per section 7.1.3.1 a zero BSIZEPACKET falls back to
> + * the RCTL.BSIZE buffer size, whose reset default (00b) is 2048
> + * bytes -- ample for the loopback frames here.
> + */
> + igb_write32(igb, E1000_SRRCTL(0), E1000_SRRCTL_DESCTYPE_ADV_ONEBUF);
> +
> + 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, E1000_TXDCTL(0)) &
> E1000_TXDCTL_QUEUE_ENABLE) &&
> + (igb_read32(igb, E1000_RXDCTL(0)) &
> E1000_RXDCTL_QUEUE_ENABLE))
> + break;
> + usleep(10);
> + }
> + VFIO_ASSERT_GE(retries, 0);
I'm not sure how I missed this in the previous iteration, but this
new-ish assert exposes a latent ordering issue on real hardware. As per
the below referenced register definitions, the per-queue enable bits are
zero until the global enable bits are set.
diff --git a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
index 97a2f29aade3..fae523059f86 100644
--- a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
+++ b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
@@ -303,16 +303,6 @@ static void igb_hw_init(struct vfio_pci_device *device)
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, E1000_TXDCTL(0)) &
E1000_TXDCTL_QUEUE_ENABLE) &&
- (igb_read32(igb, E1000_RXDCTL(0)) &
E1000_RXDCTL_QUEUE_ENABLE))
- break;
- usleep(10);
- }
- VFIO_ASSERT_GE(retries, 0);
-
/*
* Enable Receiver and Transmitter. RCTL.LBM_MAC is set in addition
* to PHY loopback as a QEMU-only accommodation: QEMU's emulated igb
@@ -335,6 +325,21 @@ static void igb_hw_init(struct vfio_pci_device *device)
igb_write32(igb, E1000_RCTL, rctl);
igb_write32(igb, E1000_TCTL, E1000_TCTL_EN | E1000_TCTL_PSP);
+ /*
+ * Wait for TX and RX queues to be enabled. Per the RXDCTL/TXDCTL
+ * register definitions (8.10.10/8.12.13), the per-queue enable bit
+ * "remains zero" until the global RCTL.RXEN/TCTL.TXEN are set, so
+ * E1000_RCTL_EN and E1000_TCTL_EN must already be written above.
+ */
+ retries = 2000;
+ while (retries-- > 0) {
+ if ((igb_read32(igb, E1000_TXDCTL(0)) &
E1000_TXDCTL_QUEUE_ENABLE) &&
+ (igb_read32(igb, E1000_RXDCTL(0)) &
E1000_RXDCTL_QUEUE_ENABLE))
+ break;
+ usleep(10);
+ }
+ VFIO_ASSERT_GE(retries, 0);
+
/*
* Program MSI-X interrupt routing per 82576 datasheet:
*
Back to 35/35 on real hardware with this. NB, patch is against fully
applied series, so lands in igb_hw_init() but the source of the
ordering issue is in patch 1 here, where it's igb_init(). Thanks,
Alex
> +
> + /*
> + * 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 = E1000_RCTL_EN | /* Receiver Enable */
> + E1000_RCTL_UPE | /* Unicast Promiscuous (for dummy MAC) */
> + E1000_RCTL_MPE | /* Multicast Promiscuous */
> + E1000_RCTL_BAM | /* Broadcast Accept Mode */
> + 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 | E1000_TCTL_PSP);
