From: Alex Williamson <[email protected]>

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 <[email protected]>
---
 .../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


Reply via email to