[PATCH 0/2] ppc/spapr: Fix MSI-X shadow and BAR mmap handling during EEH PE reset

[PATCH 0/2] ppc/spapr: Fix MSI-X shadow and BAR mmap handling during EEH PE reset

[Narayana Murty N]

This series fixes two independent but related problems in the sPAPR
EEH PE reset path for VFIO pass-through devices.

Patch 1 fixes a regression in MSI-X interrupt delivery after EEH
recovery.  The previous code called msix_reset() during PE pre-reset,
which wiped the MSI-X table shadow.  On pSeries the shadow is
populated via ibm,change-msi -> spapr_msi_setmsg() -> msix_set_message()
and is not necessarily rebuilt before the guest re-enables MSI-X.
vfio_msix_enable() -> vfio_msix_vector_do_use() re-arms KVM irqfd
routes from the existing shadow, so wiping it breaks interrupt delivery.
The fix clears only MSI-X Enable using the cached config shadow and
dispatches through pci_host_config_write_common() so that
vfio_msix_disable() tears down vectors cleanly without touching the
shadow table.

Patch 2 closes a race between the synchronous PE reset ioctl and QEMU's
direct-mapped BAR mmap windows.  While the hardware is held in PCI reset
the device BARs are inaccessible.  Any guest MMIO fault that is serviced
through an active mmap during this window can produce an indeterminate
result.  The fix disables all BAR mmaps before issuing the reset ioctl
and re-enables them after VFIO_EEH_PE_CONFIGURE succeeds.  Any pending
INTx mmap re-enable timer is also cancelled to prevent the timer from
re-arming the mmap window before configure completes.

without patch:
<snip>
[   21.549925][  T251] EEH: Beginning: 'resume'
[   21.550086][  T251] PCI 0001:00:01.0#0001: EEH: Invoking tg3->resume()
[   21.605321][  T251] PCI 0001:00:01.0#0001: EEH: tg3 driver reports: 'none'
[   21.605462][  T251] EEH: Finished:'resume'
[   21.605552][  T251] EEH: Recovery successful.
</snip>
<snip>
# ethtool enP1p0s1 |grep Link 
        Link detected: no
</snip>

with patch:
<snip>
[ 5696.920423][  T250] EEH: Beginning: 'resume'
[ 5696.920497][  T250] PCI 0001:00:01.0#0001: EEH: Invoking tg3->resume()
[ 5696.975922][  T250] PCI 0001:00:01.0#0001: EEH: tg3 driver reports: 'none'
[ 5696.976165][  T250] EEH: Finished:'resume'
[ 5696.976298][  T250] EEH: Recovery successful.
[ 5700.086095][   C28] tg3 0001:00:01.0 enP1p0s1: Link is up at 1000 Mbps, full duplex
[ 5700.086330][   C28] tg3 0001:00:01.0 enP1p0s1: Flow control is on for TX and on for RX
</snip>
<snip>
# ethtool enP1p0s1 |grep Link 
        Link detected: yes
</snip>


Narayana Murty N (2):
  ppc/spapr: Preserve MSI-X shadow across EEH PE reset
  ppc/spapr: Temporarily disable VFIO BAR mmap during EEH PE reset

 hw/ppc/spapr_pci_vfio.c | 122 ++++++++++++++++++++++++++++++++--------
 1 file changed, 100 insertions(+), 22 deletions(-)

-- 
2.54.0

[PATCH v1 1/2] ppc/spapr: Preserve MSI-X shadow across EEH PE reset

[Narayana Murty N]

On pSeries, the MSI-X table shadow in QEMU is not sourced from the
physical device.  It is populated by sPAPR RTAS through the
ibm,change-msi call path:

  ibm,change-msi
    -> spapr_msi_setmsg()
      -> msix_set_message()      /* writes each shadow entry */

As per PAPR+ §7.3.10.5.1 R1–14, the platform must restore the IOA's MSI
configuration space across a reset; the guest therefore does not
re-issue ibm,change-msi after EEH recovery, and the QEMU shadow must
survive the PE reset intact.

EEH recovery does not necessarily cause the guest to re-issue
ibm,change-msi.  The guest VFIO PCI driver restores interrupt delivery
by writing MSI-X Enable = 1 directly via config space.  QEMU then
dispatches through:

  vfio_msix_enable()
    -> vfio_msix_vector_do_use()  /* re-arms KVM irqfd from shadow */

The existing EEH pre-reset helper calls msix_reset() after clearing
MSI-X Enable.  msix_reset() zeroes every shadow entry.  Without the
shadow, vfio_msix_vector_do_use() cannot reconstruct the KVM irqfd
routes and device interrupts do not recover after EEH.

Signed-off-by: Narayana Murty N <nnmlinux@linux.ibm.com>
---
 hw/ppc/spapr_pci_vfio.c | 56 ++++++++++++++++++++++++-----------------
 1 file changed, 33 insertions(+), 23 deletions(-)

diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
index a748a0bf4c..c233822d14 100644
--- a/hw/ppc/spapr_pci_vfio.c
+++ b/hw/ppc/spapr_pci_vfio.c
@@ -25,6 +25,7 @@
 #include "hw/pci/msix.h"
 #include "hw/pci/pci_device.h"
 #include "hw/vfio/vfio-container-legacy.h"
+#include "hw/vfio/pci.h"
 #include "qemu/error-report.h"
 #include CONFIG_DEVICES /* CONFIG_VFIO_PCI */
 
@@ -233,47 +234,56 @@ int spapr_phb_vfio_eeh_get_state(SpaprPhbState *sphb, int *state)
     return RTAS_OUT_SUCCESS;
 }
 
-static void spapr_phb_vfio_eeh_clear_dev_msix(PCIBus *bus,
-                                              PCIDevice *pdev,
-                                              void *opaque)
+/*
+ * Prepare a single VFIO PCI device for an EEH PE hot or fundamental reset.
+ *
+ * On pSeries the MSI-X table shadow is populated by ibm,change-msi via
+ * spapr_msi_setmsg() -> msix_set_message().  EEH recovery does not
+ * necessarily re-issue ibm,change-msi; the guest restores MSI-X Enable
+ * directly, causing QEMU to dispatch through vfio_msix_enable() ->
+ * vfio_msix_vector_do_use(), which re-arms the KVM irqfd routes from the
+ * existing shadow entries.
+ *
+ * Therefore, msix_reset() must NOT be called here.  Calling it would wipe
+ * those shadow entries and prevent interrupt delivery after EEH recovery.
+ *
+ * Instead, clear MSI-X Enable using the cached pdev->config shadow (avoiding
+ * a read from potentially frozen device config space) and write through
+ * pci_host_config_write_common() so that the VFIO config-write handler calls
+ * vfio_msix_disable(), cleanly releasing vectors and KVM irqfd routes while
+ * leaving the shadow intact.
+ */
+static void spapr_phb_vfio_eeh_prepare_dev(PCIBus *bus,
+                                           PCIDevice *pdev,
+                                           void *opaque)
 {
-    /* Check if the device is VFIO PCI device */
-    if (!object_dynamic_cast(OBJECT(pdev), "vfio-pci")) {
+    uint16_t flags;
+
+    if (!object_dynamic_cast(OBJECT(pdev), TYPE_VFIO_PCI_DEVICE)) {
         return;
     }
 
-    /*
-     * The MSIx table will be cleaned out by reset. We need
-     * disable it so that it can be reenabled properly. Also,
-     * the cached MSIx table should be cleared as it's not
-     * reflecting the contents in hardware.
-     */
     if (msix_enabled(pdev)) {
-        uint16_t flags;
-
-        flags = pci_host_config_read_common(pdev,
-                                            pdev->msix_cap + PCI_MSIX_FLAGS,
-                                            pci_config_size(pdev), 2);
+        flags = pci_get_word(pdev->config + pdev->msix_cap + PCI_MSIX_FLAGS);
         flags &= ~PCI_MSIX_FLAGS_ENABLE;
         pci_host_config_write_common(pdev,
                                      pdev->msix_cap + PCI_MSIX_FLAGS,
                                      pci_config_size(pdev), flags, 2);
     }
-
-    msix_reset(pdev);
 }
 
-static void spapr_phb_vfio_eeh_clear_bus_msix(PCIBus *bus, void *opaque)
+static void spapr_phb_vfio_eeh_prepare_bus(PCIBus *bus, void *opaque)
 {
-       pci_for_each_device_under_bus(bus, spapr_phb_vfio_eeh_clear_dev_msix,
-                                     NULL);
+    pci_for_each_device_under_bus(bus,
+                                  spapr_phb_vfio_eeh_prepare_dev,
+                                  NULL);
 }
 
 static void spapr_phb_vfio_eeh_pre_reset(SpaprPhbState *sphb)
 {
-       PCIHostState *phb = PCI_HOST_BRIDGE(sphb);
+    PCIHostState *phb = PCI_HOST_BRIDGE(sphb);
 
-       pci_for_each_bus(phb->bus, spapr_phb_vfio_eeh_clear_bus_msix, NULL);
+    pci_for_each_bus(phb->bus, spapr_phb_vfio_eeh_prepare_bus, NULL);
 }
 
 int spapr_phb_vfio_eeh_reset(SpaprPhbState *sphb, int option)
-- 
2.51.1

[PATCH v1 2/2] ppc/spapr: Temporarily disable VFIO BAR mmap during EEH PE reset

[Narayana Murty N]

An EEH PE hot or fundamental reset involves a synchronous kernel ioctl
(VFIO_EEH_PE_RESET_HOT / VFIO_EEH_PE_RESET_FUNDAMENTAL) that asserts
then de-asserts a PCI reset signal to the endpoint.  During this window
the device's BARs are inaccessible, but QEMU may still have those BARs
memory-mapped for direct guest access.  A guest MMIO fault that arrives
while the hardware is in reset can cause an unexpected host kernel page
fault or an indeterminate read value.

Fix this by disabling the BAR mmap windows for every VFIO PCI device
under the PHB before issuing the PE reset ioctl, and re-enabling them
after VFIO_EEH_PE_CONFIGURE succeeds.

A new spapr_phb_vfio_eeh_post_configure() bus walker calls
vfio_region_mmaps_set_enabled(..., true) on the configure success
path inside spapr_phb_vfio_eeh_configure().  On failure the mmaps
remain disabled; the next EEH reset attempt will call pre_reset
again.

Signed-off-by: Narayana Murty N <nnmlinux@linux.ibm.com>
---
 hw/ppc/spapr_pci_vfio.c | 68 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/hw/ppc/spapr_pci_vfio.c b/hw/ppc/spapr_pci_vfio.c
index c233822d14..1cf058cbde 100644
--- a/hw/ppc/spapr_pci_vfio.c
+++ b/hw/ppc/spapr_pci_vfio.c
@@ -252,17 +252,28 @@ int spapr_phb_vfio_eeh_get_state(SpaprPhbState *sphb, int *state)
  * pci_host_config_write_common() so that the VFIO config-write handler calls
  * vfio_msix_disable(), cleanly releasing vectors and KVM irqfd routes while
  * leaving the shadow intact.
+ *
+ * After disabling interrupts, disable BAR mmap windows so that the host
+ * kernel PE reset ioctl does not race with QEMU direct-mapped guest accesses.
+ * The timer that would ordinarily re-enable mmaps after an INTx quiet period
+ * is cancelled here; mmaps are restored after VFIO_EEH_PE_CONFIGURE succeeds
+ * in spapr_phb_vfio_eeh_configure().
  */
 static void spapr_phb_vfio_eeh_prepare_dev(PCIBus *bus,
                                            PCIDevice *pdev,
                                            void *opaque)
 {
+    VFIOPCIDevice *vdev;
     uint16_t flags;
+    int i;
 
     if (!object_dynamic_cast(OBJECT(pdev), TYPE_VFIO_PCI_DEVICE)) {
         return;
     }
 
+    vdev = VFIO_PCI_DEVICE(pdev);
+
+    /* Step 1: disable MSI-X without wiping the shadow table (see above). */
     if (msix_enabled(pdev)) {
         flags = pci_get_word(pdev->config + pdev->msix_cap + PCI_MSIX_FLAGS);
         flags &= ~PCI_MSIX_FLAGS_ENABLE;
@@ -270,6 +281,19 @@ static void spapr_phb_vfio_eeh_prepare_dev(PCIBus *bus,
                                      pdev->msix_cap + PCI_MSIX_FLAGS,
                                      pci_config_size(pdev), flags, 2);
     }
+
+    /*
+     * Step 2: cancel any pending INTx mmap re-enable timer.  The timer is
+     * only allocated when PCI_INTERRUPT_PIN is non-zero, so guard the call.
+     */
+    if (vdev->intx.mmap_timer) {
+        timer_del(vdev->intx.mmap_timer);
+    }
+
+    /* Step 3: disable BAR mmaps last, after interrupt teardown. */
+    for (i = 0; i < PCI_ROM_SLOT; i++) {
+        vfio_region_mmaps_set_enabled(&vdev->bars[i].region, false);
+    }
 }
 
 static void spapr_phb_vfio_eeh_prepare_bus(PCIBus *bus, void *opaque)
@@ -286,6 +310,43 @@ static void spapr_phb_vfio_eeh_pre_reset(SpaprPhbState *sphb)
     pci_for_each_bus(phb->bus, spapr_phb_vfio_eeh_prepare_bus, NULL);
 }
 
+/*
+ * Re-enable BAR mmap windows for a single VFIO PCI device after a successful
+ * EEH PE configure.  Called only on the configure success path; on failure the
+ * mmaps remain disabled until the next hot/fundamental reset attempt.
+ */
+static void spapr_phb_vfio_eeh_post_configure_dev(PCIBus *bus,
+                                                   PCIDevice *pdev,
+                                                   void *opaque)
+{
+    VFIOPCIDevice *vdev;
+    int i;
+
+    if (!object_dynamic_cast(OBJECT(pdev), TYPE_VFIO_PCI_DEVICE)) {
+        return;
+    }
+
+    vdev = VFIO_PCI_DEVICE(pdev);
+
+    for (i = 0; i < PCI_ROM_SLOT; i++) {
+        vfio_region_mmaps_set_enabled(&vdev->bars[i].region, true);
+    }
+}
+
+static void spapr_phb_vfio_eeh_post_configure_bus(PCIBus *bus, void *opaque)
+{
+    pci_for_each_device_under_bus(bus,
+                                  spapr_phb_vfio_eeh_post_configure_dev,
+                                  NULL);
+}
+
+static void spapr_phb_vfio_eeh_post_configure(SpaprPhbState *sphb)
+{
+    PCIHostState *phb = PCI_HOST_BRIDGE(sphb);
+
+    pci_for_each_bus(phb->bus, spapr_phb_vfio_eeh_post_configure_bus, NULL);
+}
+
 int spapr_phb_vfio_eeh_reset(SpaprPhbState *sphb, int option)
 {
     uint32_t op;
@@ -293,6 +354,11 @@ int spapr_phb_vfio_eeh_reset(SpaprPhbState *sphb, int option)
 
     switch (option) {
     case RTAS_SLOT_RESET_DEACTIVATE:
+        /*
+         * Deactivate does not perform a full PE reset; BAR mmaps were already
+         * disabled by the preceding HOT or FUNDAMENTAL reset call and must not
+         * be re-enabled here.
+         */
         op = VFIO_EEH_PE_RESET_DEACTIVATE;
         break;
     case RTAS_SLOT_RESET_HOT:
@@ -324,6 +390,8 @@ int spapr_phb_vfio_eeh_configure(SpaprPhbState *sphb)
         return RTAS_OUT_PARAM_ERROR;
     }
 
+    spapr_phb_vfio_eeh_post_configure(sphb);
+
     return RTAS_OUT_SUCCESS;
 }
 
-- 
2.51.1