Hi Mahesh,
Thanks for the review.
On 04/10/26 9:11 PM, Mahesh J Salgaonkar wrote:
On 2026-09-02 17:17:58 Wed, Narayana Murty N wrote:
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 <[email protected]>
---
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);
+ }
Maybe you can add a small helper in something like
vfio_eeh_pci_pre_reset(pdev) in hw/vfio/pci.c to include step 2 and 3 ?
That way VFIOPCIDevice internals can be hidden under the helper.
Yes, that makes sense.
I will move the INTx mmap timer handling and BAR mmap enable/disable
into VFIO PCI helpers.
}
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);
+ }
Same here.
Also, what about the mmap timer ? Does that get re-armed when INTx is
re-initialsed during EEH recovery and hence we ignore here ?
The timer itself remains allocated; timer_del() only cancels the pending
expiration.
I don't think we need to explicitly re-arm it from the EEH
post-configure path. After VFIO_EEH_PE_CONFIGURE succeeds we restore the
BAR mmap state. If the device subsequently uses INTx, the next
vfio_intx_interrupt() will disable the mmap regions and arm the mmap
timer again when mmap_timeout is configured.
For the MSI-X case, disabling MSI-X temporarily moves VFIO back to INTx,
and when the guest restores MSI-X, the normal vfio_msix_enable() path
tears that INTx state down again.
+}
+
+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;
}
Thanks,
-Mahesh.
Thanks,
Narayana.
